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 airflow upgrade-check command (MVP) (v1-10-test) #9467

Merged
merged 9 commits into from
Sep 10, 2020

Conversation

mik-laj
Copy link
Member

@mik-laj mik-laj commented Jun 21, 2020

This is a DRAFT and I still have to complete the missing documentation and unit tests.

I wanted to prepare a minimal valuable product for the upgrade_check command. Only one rule is implemented, but I think that's enough to show the feature. In the next steps, we can think about what rules we need to create, create tickets, and then work on the next rules by the whole community.

Recently, we have a lot of new beginner contributors and I think we can try to engage them for this task. Most rules will be small and independent, so they will be great candidates for good-first-issue.

An example of the result of this command is presented in the termianlshot below
Screenshot 2020-06-13 at 12 58 38

Example JSON:

[
  {
    "rule": "ConnTypeIsNotNullableRule",
    "message": "Connection<id=''\", conn_id=AA> have empty conn_type field."
  }
]

Part of: #8765
Continuation of the change, but for 1.10: #9276

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.
  • [XI 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.

@mik-laj mik-laj changed the title [WIP] Add airflow upgrade-check command (MVP) [WIP] Add airflow upgrade-check command (MVP) (v1-10-test) Jun 21, 2020
@mik-laj mik-laj marked this pull request as draft June 22, 2020 11:26
@dimberman dimberman force-pushed the v1-10-test branch 13 times, most recently from 238d841 to 2f17299 Compare June 24, 2020 18:52
@dimberman dimberman force-pushed the v1-10-test branch 5 times, most recently from 4f848da to e0bdf95 Compare June 27, 2020 05:50
@potiuk potiuk force-pushed the v1-10-test branch 4 times, most recently from 845bcbe to 90446e5 Compare June 29, 2020 14:33
@kaxil kaxil force-pushed the v1-10-test branch 4 times, most recently from c644d53 to 5a54173 Compare July 1, 2020 01:18
@turbaszek turbaszek self-assigned this Sep 7, 2020
formatted_results = [self._info_from_rule_status(rs) for rs in rule_statuses]
with open(self.filename, "w+") as output_file:
json.dump(formatted_results, output_file, indent=2)
print("Saved result to: {}".format(self.filename))
Copy link
Member

Choose a reason for hiding this comment

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

Example result

[
  {
    "rule": "MokcRule",
    "title": "title",
    "messages": [
      "msg1",
      "msg2"
    ]
  }
]

@turbaszek
Copy link
Member

Ok, no idea how to make the test runnable on CI. Two days I had only py3.5 failing, know everything is cancelled... @potiuk any ideas?

@turbaszek
Copy link
Member

@mik-laj @kaxil @dimberman @ashb please take a look, the CI is green and I think we are good to merge it

@turbaszek
Copy link
Member

Thanks @ashb, I will just rebase the PR as I spotted the Dataproc HTTP commit here

@@ -337,7 +337,7 @@ def cancel(self, project_id, job_id, region='global'):
projectId=project_id,
region=region,
jobId=job_id
)
).execute(num_retries=self.num_retries)
Copy link
Member

Choose a reason for hiding this comment

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

yeah this one

@turbaszek turbaszek merged commit 547d03d into apache:v1-10-test Sep 10, 2020
@turbaszek turbaszek deleted the upgrade-check-mvp-110 branch September 10, 2020 14:38
@dimberman
Copy link
Contributor

@turbaszek @mik-laj is this still a draft? Should I merge this into 1.10.13?

@turbaszek
Copy link
Member

@dimberman I think we should include to in "last" 1.10 release. Currently there's only one rule implemented so not a big gain

turbaszek added a commit to PolideaInternal/airflow that referenced this pull request Oct 21, 2020
Co-authored-by: Tomek Urbaszek <tomasz.urbaszek@polidea.com>
Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
turbaszek pushed a commit to PolideaInternal/airflow that referenced this pull request Oct 23, 2020
Co-authored-by: Tomek Urbaszek <tomasz.urbaszek@polidea.com>
Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
(cherry picked from commit 2904810)
cfei18 pushed a commit to cfei18/incubator-airflow that referenced this pull request Mar 5, 2021
Co-authored-by: Tomek Urbaszek <tomasz.urbaszek@polidea.com>
Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
(cherry picked from commit 2904810)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:CLI area:upgrade Facilitating migration to a newer version of Airflow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants