-
Notifications
You must be signed in to change notification settings - Fork 177
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
Add AWS Athena profile mapping #578
Conversation
👷 Deploy Preview for amazing-pothos-a3bca0 processing.
|
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 a lot for your contribution, @benjamin-awd. It looks great!
I left minor comments. I'll approve and merge once the checks pass.
e54fb10
to
f27655a
Compare
f27655a
to
00e354c
Compare
Hi @tatiana -- changes made, let me know if there's anything else |
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #578 +/- ##
==========================================
+ Coverage 92.83% 92.90% +0.06%
==========================================
Files 49 51 +2
Lines 1983 2000 +17
==========================================
+ Hits 1841 1858 +17
Misses 142 142
☔ View full report in Codecov by Sentry. |
Thank you very much, @benjamin-awd ! If you could improve the code coverage, by addressing the following comments, it would be great. |
00e354c
to
0e08feb
Compare
I realized that the mock profile wasn't actually needed, so I removed it -- the other profile mappings that add the child mock_profile property do so in order to add specific default values. |
**Features** * Add support to model versioning available since dbt 1.6 by @binhnq94 in #516 * Add AWS Athena profile mapping by @benjamin-awd in #578 * Support customizing how dbt nodes are converted to Airflow by @tatiana in #503 * Make the arg ``dbt_project_path`` in the ``ProjectConfig`` optional by @MrBones757 in #581 **Bug fixes** * Fix Cosmos custom selector to support filtering a single model by @jlaneve and @harels in #576 * Fix using ``GoogleCloudServiceAccountDictProfileMapping`` together with ``LoadMethod.DBT_LS`` by @joppevos in #587 * Fix using the ``full_refresh`` argument in projects that contain tests by @EgorSemenov and @tatiana in #590 * Stop creating symbolic links for ``dbt_packages`` (solves ``LocalExecutor`` concurrency issue) by @tatiana in #600 **Others** * Docs: add reference to original Jaffle Shop project by @erdos2n in #583 * Docs: retries & note about DagBag error by @TJaniF in #592 * pre-commit updates in #575 and #585
@@ -21,6 +22,7 @@ | |||
from .trino.ldap import TrinoLDAPProfileMapping | |||
|
|||
profile_mappings: list[Type[BaseProfileMapping]] = [ | |||
AthenaAccessKeyProfileMapping, |
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.
Hey, should this have been included in the all export on line 60?
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.
Yeah, good spot.
I have an upcoming PR for the Athena profile mapping, so I'll probably squeeze the fix in there.
This shouldn't affect anyone, unless they're doing from cosmos.profiles import *
Adds the `aws_session_token` argument to Athena, which was added to dbt-athena 1.6.4 in dbt-labs/dbt-athena#459 Closes: #609 Also addresses this comment: #578 (comment)
Adds the `aws_session_token` argument to Athena, which was added to dbt-athena 1.6.4 in dbt-labs/dbt-athena#459 Closes: astronomer#609 Also addresses this comment: astronomer#578 (comment)
Description
This PR adds an Athena profile mapping using the
aws
connection type (unfortunately, there is no specific connection type for Athena, unlike Redshift).This PR allows the user to avoid storing sensitive credentials in a profiles.yml file on Airflow, and instead store them in Airflow as an AWS connection.
Related Issue(s)
Closes #201
Checklist