-
Notifications
You must be signed in to change notification settings - Fork 20
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
Bug/databricks get url parameter #130
Bug/databricks get url parameter #130
Conversation
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.
@fivetran-catfritz great work on this PR! I just have a few small comments and suggestions I would like you to look at and address before approving. Let me know if you have any questions!
Thanks @fivetran-joemarkiewicz! I made the changes and added comments, so it's ready for re-review. |
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 @fivetran-catfritz these changes look good to me! Let's hold off merging this to the live branch until we also incorporate the sql server updates as well.
If you have the time, would you be able to create a release branch for release/v0.4.9
. We can then merge this branch into there, then the sql server updates into that branch as well before then rolling out the update to the live branch.
What change does this PR introduce?
extract_url_parameter
to create special logic for Databricks instances not supported bydbt_utils.get_url_parameter()
. The macro usesdbt_utils.get_url_parameter()
for default, non-Databricks targets. See README for more details.If this PR introduces a new macro, how did you test the new macro?
If this PR introduces a modification to an existing macro, which packages is the macro currently present in and what steps were taken to test compatibility across packages?
fivetran_utils
macro, but is meant to replacedbt_utils.get_url_parameter()
.dbt_utils.get_url_parameter
:Did you update the README to reflect the macro addition/modifications?