Skip to content

build(pre-commit): check deferrable default value - #32370

Merged
potiuk merged 9 commits into
apache:mainfrom
astronomer:pre-commit-check-deferrable-default
Jul 7, 2023
Merged

build(pre-commit): check deferrable default value#32370
potiuk merged 9 commits into
apache:mainfrom
astronomer:pre-commit-check-deferrable-default

Conversation

@Lee-W

@Lee-WLee-W commented Jul 5, 2023

Copy link
Copy Markdown
Member

related: #31712, #32355 (comment), #29462 (review)


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@potiuk

Copy link
Copy Markdown
Member

looks good :). Let's see how it works :)

@Lee-W
Lee-Wforce-pushed the pre-commit-check-deferrable-default branch from 8ece00b to 78cf7e8CompareJuly 5, 2023 13:40
@Lee-W
Lee-W marked this pull request as ready for review July 5, 2023 14:06
@Lee-W

Lee-W commented Jul 5, 2023

Copy link
Copy Markdown
MemberAuthor

@potiuk Just finished the implementation. I use it to detect 2 operators with deferrable = False and fix in this PR.

@potiuk

Copy link
Copy Markdown
Member

@potiuk Just finished the implementation. I use it to detect 2 operators with deferrable = False and fix in this PR.

That was my hope that we will get something like that to test it :)

Comment threadscripts/ci/pre_commit/pre_commit_check_deferrable_default.py Outdated
@Lee-W
Lee-Wforce-pushed the pre-commit-check-deferrable-default branch from 47b0de9 to 1629a0fCompareJuly 6, 2023 00:30
@uranusjr
uranusjrforce-pushed the pre-commit-check-deferrable-default branch from 5a82e9a to cf31b57CompareJuly 6, 2023 03:22

@uranusjruranusjr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I added a couple of commits to make the code a slightly more robust and well formatted.

@uranusjr

Copy link
Copy Markdown
Member

One more change to make the doc URL only appear once, not for every error detected.

@potiuk

Copy link
Copy Markdown
Member

Seems the checks are failing now :(

@Lee-W
Lee-Wforce-pushed the pre-commit-check-deferrable-default branch from 73222d0 to 40430c8CompareJuly 7, 2023 04:18
@Lee-W
Lee-Wforce-pushed the pre-commit-check-deferrable-default branch from 40430c8 to 03e1174CompareJuly 7, 2023 06:33
@uranusjr
uranusjrforce-pushed the pre-commit-check-deferrable-default branch from 89bf164 to 9463da5CompareJuly 7, 2023 07:13
@potiuk
potiuk merged commit 53d9421 into apache:mainJul 7, 2023
syedahsn pushed a commit to aws-mwaa/upstream-to-airflow that referenced this pull request Jul 11, 2023
---------
Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@Lee-W@potiuk@uranusjr@vincbeck