Repository navigation
Bugfix: Add missing application item to conn params in snowflake test - #16462
ephraimbuddy wants to merge 3 commits into
Conversation
boring-cyborg
Bot
added
area:providers
provider:snowflake
|
Sorry. I was hurring too much with that :( |
| 'role': 'af_role', | ||
| 'authenticator': 'snowflake', | ||
| 'session_parameters': {"QUERY_TAG": "This is a test hook"}, | ||
| "application": os.environ.get("AIRFLOW_SNOWFLAKE_PARTNER", "AIRFLOW"), |
There was a problem hiding this comment.
IMO it’d be better to mock the environ and also test the logic is correct.
There was a problem hiding this comment.
@uranusjr I'm not sure I understand the logic you want tested. I have just mocked it
github-actions
Bot
added
the
okay to merge
|
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease. |
|
Sorry I was a bit delayed thinking of logic to check as @uranusjr advised |
The application item is missing in snowflake hook test
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.