Skip to content

Change the mock value of port in TrinoBaseProfileMapping to an integer#1322

Merged
tatiana merged 5 commits into
astronomer:mainfrom
dwolfeu:main
Nov 15, 2024
Merged

Change the mock value of port in TrinoBaseProfileMapping to an integer#1322
tatiana merged 5 commits into
astronomer:mainfrom
dwolfeu:main

Conversation

@dwolfeu
Copy link
Copy Markdown
Contributor

@dwolfeu dwolfeu commented Nov 13, 2024

Description

The mock value of port in TrinoBaseProfileMapping has been changed to an integer.

Related Issue(s)

Closes: #1315

Breaking Change?

Checklist

  • I have made corresponding changes to the documentation (if required)
  • I have added tests that prove my fix is effective or that my feature works

@dwolfeu dwolfeu marked this pull request as ready for review November 13, 2024 15:18
@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. profile:trino Related to Trino ProfileConfig labels Nov 13, 2024
@dwolfeu
Copy link
Copy Markdown
Contributor Author

dwolfeu commented Nov 13, 2024

I tested this using the local docker-compose setup, adding

apache-airflow-providers-trino
dbt-trino

to dev/requirements.txt, and the dag I included to the bug report.

Copy link
Copy Markdown
Contributor

@pankajkoti pankajkoti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Would it be possible to add a test for this that demonstrates how it would fail without this mock_profile method implementation and that with having it succeeds?

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.03%. Comparing base (9a19084) to head (ce7d46f).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1322   +/-   ##
=======================================
  Coverage   96.03%   96.03%           
=======================================
  Files          67       67           
  Lines        3983     3988    +5     
=======================================
+ Hits         3825     3830    +5     
  Misses        158      158           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment thread cosmos/profiles/trino/base.py
@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Nov 14, 2024
@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Nov 15, 2024
@tatiana tatiana merged commit 7d487a0 into astronomer:main Nov 15, 2024
@pankajkoti
Copy link
Copy Markdown
Contributor

Thanks a lot @dwolfeu for this great contribution & being so quick at contributing it as well as addressing feedback!

@dwolfeu
Copy link
Copy Markdown
Contributor Author

dwolfeu commented Nov 15, 2024

No worries. It was a pleasure. Thank you @pankajkoti and @tatiana for your help!

@tatiana tatiana added this to the Cosmos 1.8.0 milestone Dec 16, 2024
@tatiana tatiana mentioned this pull request Dec 17, 2024
tatiana added a commit that referenced this pull request Dec 20, 2024
**New Features**

* Support customizing Airflow operator arguments per dbt node by @wornjs
in #1339. [More
information](https://astronomer.github.io/astronomer-cosmos/getting_started/custom-airflow-properties.html).
* Support uploading dbt artifacts to remote cloud storages via callback
by @pankajkoti in #1389. [Read
more](https://astronomer.github.io/astronomer-cosmos/configuration/callbacks.html).
* Add support to ``TestBehavior.BUILD`` by @tatiana in #1377.
[Documentation](https://astronomer.github.io/astronomer-cosmos/configuration/testing-behavior.html).
* Add support for the "at" operator when using ``LoadMode.DBT_MANIFEST``
or ``CUSTOM`` by @benjy44 in #1372
* Add dbt clone operator by @pankajastro in #1326, as documented in
[here](https://astronomer.github.io/astronomer-cosmos/getting_started/operators.html).
* Support rendering tasks with non-ASCII characters by @t0momi219 in
#1278 [Read
more](https://astronomer.github.io/astronomer-cosmos/configuration/task-display-name.html)
* Add warning callback on source freshness by @pankajastro in #1400
[Read
more](https://astronomer.github.io/astronomer-cosmos/configuration/source-nodes-rendering.html#on-warning-callback-callback)
* Add Oracle Profile mapping by @slords and @pankajkoti in #1190 and
#1404
* Emit telemetry to Scarf during DAG run by @tatiana in #1397
* Save tasks map as ``DbtToAirflowConverter`` property by
@internetcoffeephone and @hheemskerk in #1362

**Bug Fixes**

* Fix the mock value of port in ``TrinoBaseProfileMapping`` to be an
integer by @dwolfeu #1322
* Fix access to the ``dbt docs`` menu item outside of Astro cloud by
@tatiana in #1312
* Add missing ``DbtSourceGcpCloudRunJobOperator`` in module
``cosmos.operators.gcp_cloud_run_job`` by @anai-s in #1290
* Support building ``DbtDag`` without setting paths in ``ProjectConfig``
by @tatiana in #1307
* Fix parsing dbt ls outputs that contain JSONs that are not dbt nodes
by @tatiana in #1296
* Fix Snowflake Profile mapping when using AWS default region by
@tatiana in #1406
* Fix dag rendering for taskflow + DbtTaskGroup combo by @pankajastro in
#1360

**Enhancements**

* Improve dbt command execution logs to troubleshoot ``None`` values by
@tatiana in #1392
* Add logging of stdout to dbt graph run_command by @KarolGongola in
#1390
* Save tasks map as DbtToAirflowConverter property by
@internetcoffeephone and @hheemskerk in #1362
* Support rendering build operator task-id with non-ASCII characters by
@pankajastro in #1415

**Docs**

* Remove extra ` char from docs by @pankajastro in #1345
* Add limitation about copying target dir files to remote by @pankajkoti
in #1305
* Generalise example from README by @ReadytoRocc in #1311
* Add security policy by @tatiana, @chaosmaw and @lzdanski in # 1385
* Mention in documentation that the callback functionality is supported
in ``ExecutionMode.VIRTUALENV`` by @pankajkoti in #1401

**Others**

* Restore Jaffle Shop so that ``basic_cosmos_dag`` works as documented
by @tatiana in #1374
* Remove Pytest durations from tests scripts by @tatiana in #1383
* Remove typing-extensions as dependency by @pankajastro in #1381
* Pin dbt-databricks version to < 1.9 by @pankajastro in #1376
* Refactor ``dbt-sqlite`` tests to use ``dbt-postgres`` by @pankajastro
in #1366
* Remove 'dbt-core<1.8.9' pin by @tatiana in #1371
* Remove dependency ``eval_type_backport`` by @tatiana in #1370
* Enable kubernetes tests for dbt>=1.8 by @pankajastro #1364
* CI Workaround: Pin dbt-core, Disable SQLite Tests, and Correctly
Ignore Clone Test to Pass CI by @pankajastro in #1337
* Enable Azure task in the remote store manifest example DAG by
@pankajkoti in #1333
* Enable GCP remote manifest task by @pankajastro in #1332
* Add exempt label option in GH action stale job by @pankajastro in
#1328
* Add integration test for source node rendering by @pankajastro in
#1327
* Fix vulnerability issue on docs dependency by @tatiana in #1313
* Add postgres pod status check for k8s tests in CI by @pankajkoti in
#1320
* [CI] Reduce the amount taking to run tests in the CI from 5h to 11min
by @tatiana in #1297
* Enable secret detection precommit check by @pankajastro in #1302
* Fix security vulnerability, by not pinning Airflow 2.10.0 by @tatiana
in #1298
* Fix Netlify build timeouts by @tatiana in #1294
* Add stalebot to label/close stale PRs and issues by @tatiana in #1288
* Unpin dbt-databricks version by @pankajastro in #1409
* Fix source resource type tests by @pankajastro in #1405
* Increase performance tests models by @tatiana in #1403
* Drop running 1000 models in the CI by @pankajkoti in #1411
* Fix releasing package to PyPI by @tatiana in #1396
* Pre-commit hook updates in #1394, #1373, #1358, #1340, #1331, #1314,
#1301

Co-authored-by: Pankaj Koti <pankajkoti699@gmail.com>
Co-authored-by: Pankaj Singh <pankaj.singh@astronomer.io>

Closes: #1193

---------

Co-authored-by: Pankaj Koti <pankajkoti699@gmail.com>
Co-authored-by: Pankaj Singh <98807258+pankajastro@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer profile:trino Related to Trino ProfileConfig size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] dbt ls throwing error 'mock_value' is not of type 'integer' (TrinoLDAPProfileMapping)

3 participants