You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/04/03 17:57:17 UTC

[GitHub] [airflow] potiuk opened a new pull request #22713: Prepare Breeze2 for prime time :)

potiuk opened a new pull request #22713:
URL: https://github.com/apache/airflow/pull/22713


   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.
   
   This one is based on #22695 so check only last commit
   
   <!--
   Thank you for contributing! Please make sure that your code changes
   are covered with tests. And in case of new features or big changes
   remember to adjust the documentation.
   
   Feel free to ping committers for the review!
   
   In case of existing issue, reference it using one of the following:
   
   closes: #ISSUE
   related: #ISSUE
   
   How to write a good git commit message:
   http://chris.beams.io/posts/git-commit/
   -->
   
   ---
   **^ Add meaningful description above**
   
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
   In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] eladkal commented on a change in pull request #22713: Prepare Breeze2 for prime time :)

Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #22713:
URL: https://github.com/apache/airflow/pull/22713#discussion_r841260732



##########
File path: CONTRIBUTING.rst
##########
@@ -644,7 +644,7 @@ and not packaged together with the core, unless you set ``INSTALL_PROVIDERS_FROM
 variable to ``true``.
 
 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

Review comment:
       Shouldn't it be `True`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] github-actions[bot] commented on pull request #22713: Prepare Breeze2 for prime time :)

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #22713:
URL: https://github.com/apache/airflow/pull/22713#issuecomment-1086926152


   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #22713: Prepare Breeze2 for prime time :)

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22713:
URL: https://github.com/apache/airflow/pull/22713#issuecomment-1086918758


   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #22713: Prepare Breeze2 for prime time :)

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #22713:
URL: https://github.com/apache/airflow/pull/22713#discussion_r841262750



##########
File path: CONTRIBUTING.rst
##########
@@ -644,7 +644,7 @@ and not packaged together with the core, unless you set ``INSTALL_PROVIDERS_FROM
 variable to ``true``.
 
 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

Review comment:
       Dockls are not yet updated. I leave irt for the switch :)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #22713: Prepare Breeze2 for prime time :)

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #22713:
URL: https://github.com/apache/airflow/pull/22713#discussion_r841262750



##########
File path: CONTRIBUTING.rst
##########
@@ -644,7 +644,7 @@ and not packaged together with the core, unless you set ``INSTALL_PROVIDERS_FROM
 variable to ``true``.
 
 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

Review comment:
       Dockls are not yet updated. I leave it for the switch :) (next PR)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #22713: Prepare Breeze2 for prime time :)

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #22713:
URL: https://github.com/apache/airflow/pull/22713#discussion_r841262801



##########
File path: CONTRIBUTING.rst
##########
@@ -644,7 +644,7 @@ and not packaged together with the core, unless you set ``INSTALL_PROVIDERS_FROM
 variable to ``true``.
 
 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

Review comment:
       There will be quite many changes in the docs :)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #22713: Prepare Breeze2 for prime time :)

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22713:
URL: https://github.com/apache/airflow/pull/22713#issuecomment-1086919659


   Auttocomplete: 
   
   ![image](https://user-images.githubusercontent.com/595491/161441625-662c59b8-de13-44ba-b4fc-c45af3feca07.png)
   
   Autocomplete 2:
   
   ![image](https://user-images.githubusercontent.com/595491/161441650-e9bd0b08-9017-48d4-a46f-7d32e3431ada.png)
   
   Main help with Breeze2: 
   
   ![image](https://user-images.githubusercontent.com/595491/161441686-f1ba26d1-e260-4cef-b245-c7b9e0c73ee8.png)
   
   Build image help:
   
   ![image](https://user-images.githubusercontent.com/595491/161441710-fcab8137-83c0-42bc-9dd6-572afc49f8cb.png)
   
   Start-airflow help:
   
   ![image](https://user-images.githubusercontent.com/595491/161441750-0020d889-434f-450f-9f2e-e65efc2ae48f.png)
   
   Cheatsheet:
   
   ![image](https://user-images.githubusercontent.com/595491/161441791-520207cc-3e5b-4307-96f2-ba33fac0fb3c.png)
   
   
   
   
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org