Skip to content
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

Profile name override #314

Merged
merged 6 commits into from
Jun 9, 2023
Merged

Profile name override #314

merged 6 commits into from
Jun 9, 2023

Conversation

jlaneve
Copy link
Collaborator

@jlaneve jlaneve commented Jun 6, 2023

Description

This PR allows a user to specify a profile_name_override. When specified, the profile Cosmos generates will be called the value passed.

Related Issue(s)

closes #266

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

@jlaneve jlaneve requested a review from a team as a code owner June 6, 2023 17:18
@jlaneve jlaneve requested a review from a team June 6, 2023 17:18
@netlify
Copy link

netlify bot commented Jun 6, 2023

👷 Deploy Preview for amazing-pothos-a3bca0 processing.

Name Link
🔨 Latest commit 86b817e
🔍 Latest deploy log https://app.netlify.com/sites/amazing-pothos-a3bca0/deploys/647f6a6bf3882900084f4425

@netlify
Copy link

netlify bot commented Jun 6, 2023

👷 Deploy Preview for amazing-pothos-a3bca0 processing.

Name Link
🔨 Latest commit 626aa1c
🔍 Latest deploy log https://app.netlify.com/sites/amazing-pothos-a3bca0/deploys/648219ecc336b7000848b08a

@codecov
Copy link

codecov bot commented Jun 6, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.41 🎉

Comparison is base (4e38f7b) 88.43% compared to head (626aa1c) 88.85%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #314      +/-   ##
==========================================
+ Coverage   88.43%   88.85%   +0.41%     
==========================================
  Files          41       41              
  Lines        1280     1292      +12     
==========================================
+ Hits         1132     1148      +16     
+ Misses        148      144       -4     
Impacted Files Coverage Δ
cosmos/providers/dbt/constants.py 100.00% <100.00%> (+100.00%) ⬆️
cosmos/providers/dbt/core/operators/local.py 80.34% <100.00%> (+1.16%) ⬆️
cosmos/providers/dbt/dag.py 100.00% <100.00%> (ø)
cosmos/providers/dbt/render.py 92.78% <100.00%> (+0.07%) ⬆️
cosmos/providers/dbt/task_group.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Collaborator

@tatiana tatiana left a comment

Choose a reason for hiding this comment

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

@jlaneve thanks. I left some minor comments inline!
Would this change also make sense in other execution modes (e.g. Docker and K8s)?
What happens, ATM, if the user passes this parameter to the DAG and the user is using one of these alternative execution modes?

@jlaneve jlaneve requested a review from tatiana June 8, 2023 16:21
@jlaneve
Copy link
Collaborator Author

jlaneve commented Jun 8, 2023

@tatiana ready for re-review! I addressed all your comments. This doesn't make sense to include for Docker and Kubernetes, since we don't do any profile mapping for those. If you pass in this parameter with those arguments, the DAG won't parse and it'll give you a clear error:

Screen Shot 2023-06-08 at 12 29 16 PM

Copy link
Collaborator

@tatiana tatiana left a comment

Choose a reason for hiding this comment

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

Thanks a lot, @jlaneve!

@tatiana tatiana merged commit e976a4c into main Jun 9, 2023
@tatiana tatiana deleted the profile-name-override branch June 9, 2023 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--target=dev in all environments
2 participants