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 2020/05/09 07:26:48 UTC
[GitHub] [airflow] potiuk opened a new pull request #8796: Better help on parameterized breeze commands
potiuk opened a new pull request #8796:
URL: https://github.com/apache/airflow/pull/8796
---
Make sure to mark the boxes below before creating PR: [x]
- [x] Description above provides context of the change
- [x] Unit tests coverage for changes (not needed for documentation changes)
- [x] Target Github ISSUE in description if exists
- [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [x] Relevant documentation is updated including usage instructions.
- [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
---
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/master/UPDATING.md).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #8796: Better help on parameterized breeze commands
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #8796:
URL: https://github.com/apache/airflow/pull/8796#discussion_r422526272
##########
File path: breeze
##########
@@ -535,7 +535,7 @@ function prepare_command_files() {
# Prepare script for "run test"
prepare_command_file "${BUILD_CACHE_DIR}/${LAST_DC_TEST_CI_FILE}" \
- "${DC_RUN_CI_COMMAND}" "${COMPOSE_CI_FILE}" "true" "${AIRFLOW_CI_IMAGE}" '*'
+ "${DC_RUN_CI_COMMAND}" "true" "${COMPOSE_CI_FILE}" "${AIRFLOW_CI_IMAGE}" '*'
Review comment:
I need to rebase :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #8796: Better help on parameterized breeze commands
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #8796:
URL: https://github.com/apache/airflow/pull/8796#discussion_r422527848
##########
File path: breeze
##########
@@ -804,14 +804,20 @@ function parse_arguments() {
COMMAND_TO_RUN="cleanup_image"
shift ;;
docker-compose)
- if [[ "$#" -lt 2 ]]; then
+ LAST_SUBCOMMAND="${1}"
+ if [[ $# -lt 2 ]]; then
echo "You should specify docker compose command to run"
- exit 1
+ shift
+ RUN_HELP="true"
+ else
+ DOCKER_COMPOSE_COMMAND="${2}"
+ if [[ ${DOCKER_COMPOSE_COMMAND} == "--help" ]]; then
+ RUN_HELP="true"
+ fi
Review comment:
Right. I forgot that the --options are parsed before and removed :). Good catch. Fixed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] zhongjiajie commented on a change in pull request #8796: Better help on parameterized breeze commands
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on a change in pull request #8796:
URL: https://github.com/apache/airflow/pull/8796#discussion_r422739813
##########
File path: breeze
##########
@@ -535,7 +535,7 @@ function prepare_command_files() {
# Prepare script for "run test"
prepare_command_file "${BUILD_CACHE_DIR}/${LAST_DC_TEST_CI_FILE}" \
- "${DC_RUN_CI_COMMAND}" "${COMPOSE_CI_FILE}" "true" "${AIRFLOW_CI_IMAGE}" '*'
+ "${DC_RUN_CI_COMMAND}" "true" "${COMPOSE_CI_FILE}" "${AIRFLOW_CI_IMAGE}" '*'
Review comment:
Got thanks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] zhongjiajie commented on a change in pull request #8796: Better help on parameterized breeze commands
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on a change in pull request #8796:
URL: https://github.com/apache/airflow/pull/8796#discussion_r422517722
##########
File path: breeze
##########
@@ -535,7 +535,7 @@ function prepare_command_files() {
# Prepare script for "run test"
prepare_command_file "${BUILD_CACHE_DIR}/${LAST_DC_TEST_CI_FILE}" \
- "${DC_RUN_CI_COMMAND}" "${COMPOSE_CI_FILE}" "true" "${AIRFLOW_CI_IMAGE}" '*'
+ "${DC_RUN_CI_COMMAND}" "true" "${COMPOSE_CI_FILE}" "${AIRFLOW_CI_IMAGE}" '*'
Review comment:
Why I merged the base PR but it still here
##########
File path: breeze
##########
@@ -804,14 +804,20 @@ function parse_arguments() {
COMMAND_TO_RUN="cleanup_image"
shift ;;
docker-compose)
- if [[ "$#" -lt 2 ]]; then
+ LAST_SUBCOMMAND="${1}"
+ if [[ $# -lt 2 ]]; then
echo "You should specify docker compose command to run"
- exit 1
+ shift
+ RUN_HELP="true"
+ else
+ DOCKER_COMPOSE_COMMAND="${2}"
+ if [[ ${DOCKER_COMPOSE_COMMAND} == "--help" ]]; then
+ RUN_HELP="true"
+ fi
Review comment:
I test in local and seem delete this three code and `--help` option still work, also the short option `-h`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #8796: Better help on parameterized breeze commands
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #8796:
URL: https://github.com/apache/airflow/pull/8796#discussion_r422630915
##########
File path: BREEZE.rst
##########
@@ -1046,15 +1046,21 @@ This is the current syntax for `./breeze <./breeze>`_:
Detailed usage for command: static-check
- breeze [FLAGS] static-check -- <EXTRA_ARGS>
+ breeze [FLAGS] static-check <STATIC_CHECK> -- <EXTRA_ARGS>
Run selected static checks for currently changed files. You should specify static check that
you would like to run or 'all' to run all checks. One of:
- all all-but-pylint bat-tests check-apache-license check-executables-have-shebangs
- check-hooks-apply check-merge-conflict check-xml debug-statements doctoc
- detect-private-key end-of-file-fixer flake8 forbid-tabs insert-license
- lint-dockerfile mixed-line-ending mypy pylint pylint-test setup-order shellcheck
+ all all-but-pylint airflow-config-yaml base-operator bat-tests build
+ build-providers-dependencies check-apache-license check-executables-have-shebangs
+ check-hooks-apply check-integrations check-merge-conflict check-xml
+ consistent-pylint daysago-import-check debug-statements detect-private-key doctoc
+ end-of-file-fixer fix-encoding-pragma flake8 forbid-tabs
+ incorrect-use-of-LoggingMixin insert-license isort lint-dockerfile mixed-line-ending
+ mypy provide-create-sessions pydevd pylint pylint-tests python-no-log-warn
+ rst-backticks setup-order shellcheck stylelint trailing-whitespace
+ update-breeze-file update-extras update-local-yml-file update-setup-cfg-file
+ yamllint
Review comment:
Does it work with autocomplete?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #8796: Better help on parameterized breeze commands
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #8796:
URL: https://github.com/apache/airflow/pull/8796#discussion_r422677889
##########
File path: BREEZE.rst
##########
@@ -1046,15 +1046,21 @@ This is the current syntax for `./breeze <./breeze>`_:
Detailed usage for command: static-check
- breeze [FLAGS] static-check -- <EXTRA_ARGS>
+ breeze [FLAGS] static-check <STATIC_CHECK> -- <EXTRA_ARGS>
Run selected static checks for currently changed files. You should specify static check that
you would like to run or 'all' to run all checks. One of:
- all all-but-pylint bat-tests check-apache-license check-executables-have-shebangs
- check-hooks-apply check-merge-conflict check-xml debug-statements doctoc
- detect-private-key end-of-file-fixer flake8 forbid-tabs insert-license
- lint-dockerfile mixed-line-ending mypy pylint pylint-test setup-order shellcheck
+ all all-but-pylint airflow-config-yaml base-operator bat-tests build
+ build-providers-dependencies check-apache-license check-executables-have-shebangs
+ check-hooks-apply check-integrations check-merge-conflict check-xml
+ consistent-pylint daysago-import-check debug-statements detect-private-key doctoc
+ end-of-file-fixer fix-encoding-pragma flake8 forbid-tabs
+ incorrect-use-of-LoggingMixin insert-license isort lint-dockerfile mixed-line-ending
+ mypy provide-create-sessions pydevd pylint pylint-tests python-no-log-warn
+ rst-backticks setup-order shellcheck stylelint trailing-whitespace
+ update-breeze-file update-extras update-local-yml-file update-setup-cfg-file
+ yamllint
Review comment:
Yes. that's the whole point of having ti as pre-commit does not have auto-complete.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #8796: Better help on parameterized breeze commands
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #8796:
URL: https://github.com/apache/airflow/pull/8796#discussion_r422530223
##########
File path: breeze
##########
@@ -804,14 +804,20 @@ function parse_arguments() {
COMMAND_TO_RUN="cleanup_image"
shift ;;
docker-compose)
- if [[ "$#" -lt 2 ]]; then
+ LAST_SUBCOMMAND="${1}"
+ if [[ $# -lt 2 ]]; then
echo "You should specify docker compose command to run"
- exit 1
+ shift
+ RUN_HELP="true"
+ else
+ DOCKER_COMPOSE_COMMAND="${2}"
+ if [[ ${DOCKER_COMPOSE_COMMAND} == "--help" ]]; then
+ RUN_HELP="true"
+ fi
Review comment:
I actually found that there was also one thing missing (specification of the command parameter for parameterized command). I added it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org