-
Notifications
You must be signed in to change notification settings - Fork 170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added new profile mapping configuration for Teradata #1077
Conversation
✅ Deploy Preview for sunny-pastelito-5ecb04 canceled.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, @sc250072 , thank you very much for adding support to Teradata! I added some minor comments in-line
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1077 +/- ##
==========================================
+ Coverage 96.12% 96.14% +0.02%
==========================================
Files 62 63 +1
Lines 3249 3266 +17
==========================================
+ Hits 3123 3140 +17
Misses 126 126 ☔ View full report in Codecov by Sentry. |
1. Removed mock_profile as it inherits the parent mock_profile 2. Removed TYPE_CHECKING
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @sc250072 for being so quick at addressing the comments. Happy for us to merge this PR once all the checks pass
Thank you for the quick response on the PR. Right now, I'm conducting integration testing by manually installing the .whl file on my local machine. Could you please provide any documentation or guidance on implementing integration tests that can be run through GitHub workflows? |
We have test file https://github.com/astronomer/astronomer-cosmos/blob/main/tests/test_example_dags.py which run some dags of this repo https://github.com/astronomer/astronomer-cosmos/tree/main/dev/dags also you can find some tests decorated with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Co-authored-by: Pankaj Singh <[email protected]>
@sc250072 complemeting @pankajastro 's message: #1077 (comment) Could the Teradata team share specific credentials to run integration tests from the Cosmos repo to validate that these tests pass? In that case, we'd be happy to remove the community flag. As @pankajastro mentioned, we could either have an example DAG with Teradata or other tests marked with integration. Another alternative, similar to Airflow, is to run a Cosmos DAG using this profile in your own CI/CD against the Cosmos main branch and share a dashboard/page with us with the job status (if you could extend https://teradata.github.io/airflow/. It would be awesome). In this case, you wouldn't have to share credentials with us, and we could help guarantee the stability of this feature. In this case, we could remove the |
@sc250072 this work looks great, we'll be merging this PR and we can address removing the |
@pankajastro @tatiana any expected date of new version of astronomer-cosmos, which includes teradata support. https://github.com/astronomer/astronomer-cosmos/blob/main/CHANGELOG.rst |
@sc250072 you can validate the feature in Cosmos 1.6.0a1 |
@tatiana I have tested Cosmos 1.6.0a1 and identified an issue with the Teradata mock profile, documented here: #1087. I have resolved this issue and submitted a pull request at #1088. After addressing the issue, I validated the fix using a locally built .whl file and tested it with several Cosmos DAGs within the dbt-jaffle-shop project, utilizing the dbt-teradata adapter. Everything is functioning correctly now. Could we proceed with another alpha release incorporating these changes?" |
Hi @sc250072 , thanks for all the work on this! Yes, we'll be releasing a new alpha this week Nd we'll let you know |
Teradata has [Provider](https://airflow.apache.org/docs/apache-airflow-providers-teradata/stable/index.html) in airflow and [adapter](https://github.com/Teradata/dbt-teradata) in dbt. The cosmos library doesn't have profile configuration with mapping support. This PR address this issue. Closes: astronomer#1053
New Features * Add support for loading manifest from cloud stores using Airflow Object Storage by @pankajkoti in #1109 * Cache ``package-lock.yml`` file by @pankajastro in #1086 * Support persisting the ``LoadMode.VIRTUALENV`` directory by @tatiana in #1079 * Add support to store and fetch ``dbt ls`` cache in remote stores by @pankajkoti in #1147 * Add default source nodes rendering by @arojasb3 in #1107 * Add Teradata ``ProfileMapping`` by @sc250072 in #1077 Enhancements * Add ``DatabricksOauthProfileMapping`` profile by @CorsettiS in #1091 * Use ``dbt ls`` as the default parser when ``profile_config`` is provided by @pankajastro in #1101 * Add task owner to dbt operators by @wornjs in #1082 * Extend Cosmos custom selector to support + when using paths and tags by @mvictoria in #1150 * Simplify logging by @dwreeves in #1108 Bug fixes * Fix Teradata ``ProfileMapping`` target invalid issue by @sc250072 in #1088 * Fix empty tag in case of custom parser by @pankajastro in #1100 * Fix ``dbt deps`` of ``LoadMode.DBT_LS`` should use ``ProjectConfig.dbt_vars`` by @tatiana in #1114 * Fix import handling by lazy loading hooks introduced in PR #1109 by @dwreeves in #1132 * Fix Airflow 2.10 regression and add Airflow 2.10 in test matrix by @pankajastro in #1162 Docs * Fix typo in azure-container-instance docs by @pankajastro in #1106 * Use Airflow trademark as it has been registered by @pankajastro in #1105 Others * Run some example DAGs in Kubernetes execution mode in CI by @pankajastro in #1127 * Install requirements.txt by default during dev env spin up by @@CorsettiS in #1099 * Remove ``DbtGraph.current_version`` dead code by @tatiana in #1111 * Disable test for Airflow-2.5 and Python-3.11 combination in CI by @pankajastro in #1124 * Pre-commit hook updates in #1074, #1113, #1125, #1144, #1154, #1167 --------- Co-authored-by: Pankaj Koti <[email protected]> Co-authored-by: Pankaj Singh <[email protected]>
Description
Teradata has Provider in airflow and adapter in dbt. The cosmos library doesn't have profile configuration with mapping support. This PR address this issue.
Related Issue(s)
Closes #1053
Breaking Change?
No Breaking Change.
Checklist