-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Prepare Breeze2 for prime time :) #22713
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
Conversation
|
cc: @Bowrna @edithturn -> I have deeeply reviewed, tested and cleaned up the changes. This is really cool what we have, after I removed some duplications, cleaned up some naming, added some docstrings and reviewed and made sure all the parameters we have, we are 100% ready to replace old breeze and move the old breeze to legacy. This will be the next PR after the #22695 and this one is merged. My changes were mostly about beautifying things, refactoring/shuffling them around so that they make more sense and making sure we have no duplications. All the rest remained largely as You've implemented it. FANTASTIC JOB. Some cool screenshots follow. |
eladkal
left a comment
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.
🎉🎉🎉🎉
|
|
||
| In Breeze - which is a development environment, ``INSTALL_PROVIDERS_FROM_SOURCES`` variable is set to true, | ||
| but you can add ``--skip-installing-airflow-providers-from-sources`` flag to Breeze to skip installing providers when | ||
| but you can add ``--install-providers-from-sources=true`` flag to Breeze to skip installing providers when |
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.
Shouldn't it be True?
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.
Dockls are not yet updated. I leave it for the switch :) (next PR)
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.
There will be quite many changes in the docs :)
|
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease. |
7b1d0b3 to
c0645d9
Compare
This is a review and clean-up for all the parameters and commands for Breeze2 in order to prepare it for being used by the contribugors. There are various small fixes here and there, removal of duplicated code, refactoring and moving code around as well as cleanup and review all the parameters used for all implemented commands. The parameters, default values and their behaviours were updated to match "new" life of Breeze rather than old one. Some improvements are made to the autocomplete and click help messages printed. Full list of choices is always displayed, parameters are groups according to their target audience, and they were sorted according to importance and frequency of use. Various messages have been colourised according to their meaning - warnings as yellow, errors as red and informational messages as bright_blue. The `dry-run` option has been added to just show what would have been run without actually running some potentially "write" commands (read commands are still executed) so that you can easily verify and manually copy and execute the commands with option to modify them before. The `dry_run` and `verbose` options are now used for all commands. The "main" command now runs "shell" by default similarly as the original Breeze. All "shortcut" parameters have been standardized - i.e common options (verbose/dry run/help) have one and all common flags that are likely to be used often have an assigned shortcute. The "stop" and "cleanup" command have been added as they are necessary for average user to complete the regular usage cycle. Documentation for all the important methods have been updated.
c0645d9 to
364f2aa
Compare
|
wow wow, Breeze looks really amazing, @potiuk 👏🏼 |
| }, | ||
| ] | ||
| } | ||
|
|
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.
Woah this is really cool :)
|
Indeed. This week we are going to switch everyone to it :) |
|
And the code is now structured for implementing the rest much better :). I had a lot of fun refactoring and moving parts (with the help of IntelliJ' refactoring features) and it was already very clean and well structured so I mostly just shuffeld things around, made some cleanups and common code extraction and clarified the use of some parameters (removed some of them or simplified them a little. That was mostly it :) |
|
Please take a look at the structure and docs I added - to familarise a bit with the new structure (thought the code is mostly the same, just you will find it in new places sometimes ;) |






This is a review and clean-up for all the parameters and
commands for Breeze2 in order to prepare it for being
used by the contribugors.
There are various small fixes here and there, removal
of duplicated code, refactoring and moving code around
as well as cleanup and review all the parameters used
for all implemented commands.
The parameters, default values and their behaviours were
updated to match "new" life of Breeze rather than old
one.
Some improvements are made to the autocomplete and
click help messages printed. Full list of choices is
always displayed, parameters are groups according to
their target audience, and they were sorted according
to importance and frequency of use.
Various messages have been colourised according to their
meaning - warnings as yellow, errors as red and
informational messages as bright_blue.
The
dry-runoption has been added to just show whatwould have been run without actually running some
potentially "write" commands (read commands are still
executed) so that you can easily verify and manually
copy and execute the commands with option to modify
them before. The
dry_runandverboseoptions arenow used for all commands.
The "main" command now runs "shell" by default similarly
as the original Breeze.
All "shortcut" parameters have been standardized - i.e
common options (verbose/dry run/help) have one and all
common flags that are likely to be used often have an
assigned shortcute.
The "stop" and "cleanup" command have been added
as they are necessary for average user to complete the
regular usage cycle.
Documentation for all the important methods have been
updated.
This one is based on #22695 so check only last commit
^ 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.