Skip to content

Add possibility to configure upgrade_check command #12657

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.

Sign up for GitHub

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

Jump to bottom
Merged
turbaszek merged 4 commits into from
Nov 30, 2020
Merged

Add possibility to configure upgrade_check command #12657

turbaszek merged 4 commits into from
Nov 30, 2020

Conversation

@turbaszek
Copy link
Member

@turbaszek turbaszek commented Nov 27, 2020 •
edited
Loading

In some cases users may want to suppress some checks. This commit
adds possibility to specify simple yaml file where users can
specify ignored checks and module path to custom checks.

I also disable logging to avoid output between lines of check.

Before:

root@5d103907dca8:/opt/airflow# airflow upgrade_check -i LegacyUIDeprecated -i ConnTypeIsNotNullableRule -i TaskHandlersMovedRule -i VersionCheckRule -i MesosExecutorRemovedRule

====================================================================================================== STATUS ======================================================================================================

Remove airflow.AirflowMacroPlugin class...................................................................................................................................................................SUCCESS
Chain between DAG and operator not allowed................................................................................................................................................................SUCCESS
Connection.conn_id is not unique..........................................................................................................................................................................SUCCESS
[2020-11-27 14:09:31,544] {__init__.py:50} INFO - Using executor LocalExecutor
[2020-11-27 14:09:31,546] {dagbag.py:417} INFO - Filling up the DagBag from /root
Ensure users are not using custom metaclasses in custom operators.........................................................................................................................................SUCCESS
Fernet is enabled by default..............................................................................................................................................................................SUCCESS
GCP service account key deprecation.......................................................................................................................................................................SUCCESS
Changes in import paths of hooks, operators, sensors and others...........................................................................................................................................SUCCESS
Logging configuration has been moved to new section.......................................................................................................................................................SUCCESS
Users must set a kubernetes.pod_template_file value.......................................................................................................................................................FAIL
SendGrid email uses old airflow.contrib module............................................................................................................................................................SUCCESS
Jinja Template Variables cannot be undefined..............................................................................................................................................................SUCCESS
Found 1 problem.

After:

root@5d103907dca8:/opt/airflow# airflow upgrade_check -i LegacyUIDeprecated -i ConnTypeIsNotNullableRule -i TaskHandlersMovedRule -i VersionCheckRule -i MesosExecutorRemovedRule

====================================================================================================== STATUS ======================================================================================================

Remove airflow.AirflowMacroPlugin class...................................................................................................................................................................SUCCESS
Chain between DAG and operator not allowed................................................................................................................................................................SUCCESS
Connection.conn_id is not unique..........................................................................................................................................................................SUCCESS
Ensure users are not using custom metaclasses in custom operators.........................................................................................................................................SUCCESS
Fernet is enabled by default..............................................................................................................................................................................SUCCESS
GCP service account key deprecation.......................................................................................................................................................................SUCCESS
Changes in import paths of hooks, operators, sensors and others...........................................................................................................................................SUCCESS
Logging configuration has been moved to new section.......................................................................................................................................................SUCCESS
Users must set a kubernetes.pod_template_file value.......................................................................................................................................................FAIL
SendGrid email uses old airflow.contrib module............................................................................................................................................................SUCCESS
Jinja Template Variables cannot be undefined..............................................................................................................................................................SUCCESS
Found 1 problem.

And using custom rules:

root@5d103907dca8:/opt/airflow# airflow upgrade_check -i TaskHandlersMovedRule -i VersionCheckRule -i MesosExecutorRemovedRule --config=/files/upgrade.yaml
Using config file from: /files/upgrade.yaml

====================================================================================================== STATUS ======================================================================================================

Remove airflow.AirflowMacroPlugin class...................................................................................................................................................................SUCCESS
Chain between DAG and operator not allowed................................................................................................................................................................SUCCESS
Connection.conn_id is not unique..........................................................................................................................................................................SUCCESS
Ensure users are not using custom metaclasses in custom operators.........................................................................................................................................SUCCESS
Fernet is enabled by default..............................................................................................................................................................................SUCCESS
GCP service account key deprecation.......................................................................................................................................................................SUCCESS
Changes in import paths of hooks, operators, sensors and others...........................................................................................................................................SUCCESS
Logging configuration has been moved to new section.......................................................................................................................................................SUCCESS
SendGrid email uses old airflow.contrib module............................................................................................................................................................SUCCESS
Jinja Template Variables cannot be undefined..............................................................................................................................................................SUCCESS
A custom rule.............................................................................................................................................................................................SUCCESS

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

@turbaszek turbaszek added the area:upgrade Facilitating migration to a newer version of Airflow label Nov 27, 2020
@turbaszek turbaszek force-pushed the add-ignore-flag-upgrade branch from 8a05beb to 5e9d418 Compare November 27, 2020 14:21
@github-actions
Copy link

github-actions bot commented Nov 27, 2020

The PR needs to run all tests because it modifies core of Airflow! Please rebase it to latest master or ask committer to re-run it!

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Nov 27, 2020
@turbaszek turbaszek force-pushed the add-ignore-flag-upgrade branch from 5e9d418 to 298ff1d Compare November 27, 2020 16:33
@turbaszek turbaszek changed the title Add --ignore flag to upgrade_check command @turbaszek Add possibility to configure upgrade_check command Nov 27, 2020
@turbaszek turbaszek mentioned this pull request Nov 27, 2020
Merged
@turbaszek turbaszek changed the title @turbaszek Add possibility to configure upgrade_check command Add possibility to configure upgrade_check command Nov 27, 2020
potiuk
potiuk approved these changes Nov 27, 2020
View reviewed changes
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

NICE

ashb
ashb reviewed Nov 27, 2020
View reviewed changes
airflow/upgrade/checker.py Outdated
Copy link
Member

@ashb ashb Nov 27, 2020

Choose a reason for hiding this comment

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

You're going to have to explain why this line is here.

Copy link
Member

@ashb ashb Nov 27, 2020

Choose a reason for hiding this comment

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

And if f47efe9#diff-62e7779cba210540c1bb87f26ce56a6176941d024635c606bcf360c14b4a9e12 isn't needed anymore as a result then you should remove it

Copy link
Member Author

@turbaszek turbaszek Nov 28, 2020

Choose a reason for hiding this comment

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

You're going to have to explain why this line is here.

@ashb should I add comment in code?

Copy link
Member

@ashb ashb Nov 28, 2020

Choose a reason for hiding this comment

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

Please

Copy link
Member Author

@turbaszek turbaszek Nov 28, 2020

Choose a reason for hiding this comment

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

Setting disable to ERROR will suppress the dagbag errors so I removed the logic from undefined jinja rule

ashb
ashb reviewed Nov 27, 2020
View reviewed changes
airflow/upgrade/checker.py Outdated
Copy link
Member

@ashb ashb Nov 27, 2020

Choose a reason for hiding this comment

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

How about you make remove_ignored_rules a function in here, and then it can be used for both args.ignore and the value from the config file.

Copy link
Member Author

@turbaszek turbaszek Nov 28, 2020

Choose a reason for hiding this comment

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

Refactored the logic a little bit, let me know what you think

mik-laj
mik-laj reviewed Nov 28, 2020
View reviewed changes
@turbaszek turbaszek requested review from ashb and mik-laj November 28, 2020 13:22
@turbaszek
Add possibility to configure upgrade_check command
3486d70 
In some cases users may want to suppress some checks. This commit
adds possibility to specify simple yaml file where users can
specify ignored checks and module path to custom checks.
@turbaszek
fixup! Add possibility to configure upgrade_check command
2720e19
@turbaszek turbaszek force-pushed the add-ignore-flag-upgrade branch from 7240edf to 2720e19 Compare November 28, 2020 19:09
@turbaszek
Copy link
Member Author

turbaszek commented Nov 29, 2020

It seems that tests/upgrade/rules/test_undefined_jinja_varaibles.py::TestUndefinedJinjaVariablesRule::test_invalid_check is little bit flaky

@turbaszek turbaszek requested a review from kaxil November 29, 2020 10:40
@turbaszek turbaszek merged commit fb63fbc into apache:v1-10-stable Nov 30, 2020
@turbaszek turbaszek deleted the add-ignore-flag-upgrade branch November 30, 2020 12:25
@madison-ookla madison-ookla mentioned this pull request Dec 29, 2020
Merged
cfei18 pushed a commit to cfei18/incubator-airflow that referenced this pull request Mar 5, 2021
@turbaszek
Add possibility to configure upgrade_check command (apache#12657)
c5cc1a7 
* Add possibility to configure upgrade_check command

In some cases users may want to suppress some checks. This commit
adds possibility to specify simple yaml file where users can
specify ignored checks and module path to custom checks.

* Mark TestUndefinedJinjaVariablesRule::test_invalid_check as quarantined
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@potiuk potiuk potiuk approved these changes

@mik-laj mik-laj mik-laj approved these changes

@dimberman dimberman Awaiting requested review from dimberman

@ashb ashb Awaiting requested review from ashb

@kaxil kaxil Awaiting requested review from kaxil

Assignees

No one assigned

Labels

area:upgrade Facilitating migration to a newer version of Airflow full tests needed We need to run full set of tests for this PR to merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@turbaszek @ashb @potiuk @mik-laj