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

Add check for pre-2.0 style hostname_callable config value #8637

Merged
merged 4 commits into from
May 1, 2020
Merged

Add check for pre-2.0 style hostname_callable config value #8637

merged 4 commits into from
May 1, 2020

Conversation

dimberman
Copy link
Contributor

@dimberman dimberman commented Apr 30, 2020

This lets us pull the change back in to 1.10.x, so that by the time 2.0 is around people will have had time and notice to update, without reading (the now quite long) UPDATING.md.

Depends on #8463


Make sure to mark the boxes below before creating PR: [x]

  • Description above provides context of the change
  • Unit tests coverage for changes (not needed for documentation changes)
  • Target Github ISSUE in description if exists
  • Commits follow "How to write a good git commit message"
  • Relevant documentation is updated including usage instructions.
  • I will engage committers as explained in Contribution Workflow Example.

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.
Read the Pull Request Guidelines for more information.

…old pattern in the val. Updates 'hostname_callable'.
@dimberman dimberman changed the title Changes deprecated config check rules. Now uses regex to look for an … Changes deprecated config check rules. Apr 30, 2020
@dimberman
Copy link
Contributor Author

@jhtimmins could you please explain a bit more what this PR does and what bug it addresses?

@jhtimmins
Copy link
Contributor

@dimberman This addresses an incompatibility between versions 1.10 and 2. For the hostname_callable config value, the notation changed from using colons (socket:getfqdn) to periods (socket.getfqdn).

This is a problem if you already have an airflow.cfg file (or unittest.cfg) from < 2.0, as airflow will continue to use the out-of-date values and throw an error. This branch does the following:

  1. Adds shim code to replace the colon w/ a period.
  2. Modifies the way that deprecated config values are swapped out. Previously, they did it with simple string comparison. This uses a regex to search for and replace the values, because it's possible to use a custom hostname_callable value that is formatted in the deprecated manner. Using a regex gives more control about what constitutes a deprecated value.
  3. Adds FutureWarning alert when the deprecated style is in use.

@ashb
Copy link
Member

ashb commented Apr 30, 2020

This lets us pull the change back in to 1.10.x, so that by the time 2.0 is around people will have had time and notice to update, without reading (the now quite long) UPDATING.md. Nice

airflow/configuration.py Outdated Show resolved Hide resolved
airflow/configuration.py Outdated Show resolved Hide resolved
airflow/configuration.py Outdated Show resolved Hide resolved
@ashb ashb changed the title Changes deprecated config check rules. Add check for pre-2.0 style hostname_callable value Apr 30, 2020
@ashb ashb changed the title Add check for pre-2.0 style hostname_callable value Add check for pre-2.0 style hostname_callable config value Apr 30, 2020
@ashb
Copy link
Member

ashb commented Apr 30, 2020

(Don't merge this yet, Github Actions haven't run yet)

@jhtimmins
Copy link
Contributor

Depends on #8463. @dimberman mind adding that to the description?

tests/test_configuration.py Outdated Show resolved Hide resolved
airflow/configuration.py Outdated Show resolved Hide resolved
airflow/configuration.py Outdated Show resolved Hide resolved
tests/test_configuration.py Outdated Show resolved Hide resolved
@ashb ashb merged commit ce50538 into apache:master May 1, 2020
@ashb ashb added this to the Airflow 1.10.11 milestone Jun 10, 2020
@kaxil kaxil modified the milestones: Airflow 1.10.12, Airflow 1.10.13 Aug 3, 2020
@kaxil kaxil removed this from the Airflow 1.10.13 milestone Nov 19, 2020
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.

4 participants