-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
Make static checks generated file more stable across the board #29080
Merged
+203
−387
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
potiuk
commented
Jan 21, 2023
elif stripped_line.startswith("from _typeshed import Incomplete"): | ||
new_lines.append(line + " # noqa: F401") | ||
elif stripped_line.startswith("from typing import") and "Union" not in line: | ||
new_lines.append(line + ", Union") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hack. But works (until we have single-line import here of course)
WAT ? |
Strange we have DIFFERENT version here..... aaaaaah I know. |
potiuk
force-pushed
the
improve-common-sql-stub-generation
branch
from
January 21, 2023 12:00
d8d2400
to
7c33b25
Compare
potiuk
changed the title
Improve generation of common.sql stubs
Make static checks generated file more stable accross the board
Jan 21, 2023
OK. I think I solved ALL the problems! |
There were couple of problems with static checks generating source files including generated stubs in the common.sql package: * black formatting was implemented in multiple separate scripts making it harded to fix problems in all of them * generated stub files were not formatted with is_pyi=True and black had no way to figure it out because it was working on strings * black formatting was not consistently applied in all places * EOL at the end of generated stub file was missing, leading to EOL fixer adding them after generation leading to multiple pre-commit passes needed * there was (already unused) deprecated dev dict generator that used its own black formatting. There were also couple of problems with the files generated by stubgen itself: * Union was missing in the generated stubs (this is a known issue with stubgen: python/mypy#12929 * Intellij complained on Incomplete import from _typeshed missing This PR fixes all the problems: * black formatting is now consistenly extracted and applied everywhere * when needed, is_pyi flag is passed to black so that it knows that .pyi file is being fomratted * EOL is added at the end of file when the file is generated * Union is added to the generated stub * noqa is added to _typeshed import * the dict generator is removed As the end result, generated stub files are fully importable (no errors reported by IntelliJ IDE) and consistently formatted every time.
potiuk
force-pushed
the
improve-common-sql-stub-generation
branch
from
January 21, 2023 12:42
7c33b25
to
4451c23
Compare
potiuk
changed the title
Make static checks generated file more stable accross the board
Make static checks generated file more stable across the board
Jan 22, 2023
ashb
approved these changes
Jan 23, 2023
potiuk
added a commit
to potiuk/airflow
that referenced
this pull request
Jan 26, 2023
The recent change apache#29080 introduced missing black import in the pre-commit that run the compilation. The compilation happened in the background thread and it's ouptut was only visible in the asset_compilation output file This PR fixes the black import problem by removing unnecessary import, but it will also stop start-airflow and exit with error as well as surface the output of asset compilation to the console.
potiuk
added a commit
that referenced
this pull request
Jan 26, 2023
The recent change #29080 introduced missing black import in the pre-commit that run the compilation. The compilation happened in the background thread and it's ouptut was only visible in the asset_compilation output file This PR fixes the black import problem by removing unnecessary import, but it will also stop start-airflow and exit with error as well as surface the output of asset compilation to the console.
ephraimbuddy
pushed a commit
that referenced
this pull request
Mar 8, 2023
There were couple of problems with static checks generating source files including generated stubs in the common.sql package: * black formatting was implemented in multiple separate scripts making it harded to fix problems in all of them * generated stub files were not formatted with is_pyi=True and black had no way to figure it out because it was working on strings * black formatting was not consistently applied in all places * EOL at the end of generated stub file was missing, leading to EOL fixer adding them after generation leading to multiple pre-commit passes needed * there was (already unused) deprecated dev dict generator that used its own black formatting. There were also couple of problems with the files generated by stubgen itself: * Union was missing in the generated stubs (this is a known issue with stubgen: python/mypy#12929 * Intellij complained on Incomplete import from _typeshed missing This PR fixes all the problems: * black formatting is now consistenly extracted and applied everywhere * when needed, is_pyi flag is passed to black so that it knows that .pyi file is being fomratted * EOL is added at the end of file when the file is generated * Union is added to the generated stub * noqa is added to _typeshed import * the dict generator is removed As the end result, generated stub files are fully importable (no errors reported by IntelliJ IDE) and consistently formatted every time. (cherry picked from commit 129f082)
ephraimbuddy
pushed a commit
that referenced
this pull request
Mar 8, 2023
The recent change #29080 introduced missing black import in the pre-commit that run the compilation. The compilation happened in the background thread and it's ouptut was only visible in the asset_compilation output file This PR fixes the black import problem by removing unnecessary import, but it will also stop start-airflow and exit with error as well as surface the output of asset compilation to the console. (cherry picked from commit c231f11)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There were couple of problems with static checks generating source
files including generated stubs in the common.sql package:
making it harded to fix problems in all of them
black had no way to figure it out because it was working on strings
fixer adding them after generation leading to multiple pre-commit
passes needed
its own black formatting.
There were also couple of problems with the files generated by
stubgen itself:
with stubgen: stubgen: import
Union
if generated stub code includes python/mypy#12929This PR fixes all the problems:
that .pyi file is being fomratted
As the end result, generated stub files are fully importable
(no errors reported by IntelliJ IDE) and consistently formatted
every time.
^ 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.