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

Adding Variable.update method and improving detection of variable key collisions #18159

Merged
merged 2 commits into from
Sep 12, 2021

Conversation

SamWheating
Copy link
Contributor

Re: Discussion in #17889

A few changes in this PR:

  • Update Variable.set() method to take a description argument.
  • Update Variable.setdefault() method to take a description argument.
  • Adding a Variable.update() method which will throw a KeyError if the Variable doesn't exist, and an AttributeError if it doesn't exist in the Database (since a non-metastore Variable can't be modified)
  • Improved logging around key collisions between different variable backends.
  • Updated documentation to warn users about key collisions between variable backends.

If a user has a duplicated key in the metastore and an extra secrets backend, then updates to the Variable will update the value in the metastore, but reads will read the value in the additional backend.

This is still the case, but I've improved the logging when this happens and updated the documentation to warn users about this behaviour.


^ 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.

@SamWheating SamWheating changed the title Adding Variable.update() method and improving detection of variable key collisions Adding Variable.update method and improving detection of variable key collisions Sep 10, 2021
@github-actions
Copy link

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Sep 12, 2021
@potiuk potiuk merged commit d6e7c45 into apache:main Sep 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:secrets full tests needed We need to run full set of tests for this PR to merge kind:documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants