[GitHub] [airflow] dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#discussion_r312721078 ## File path: CONTRIBUTING.md ## @@ -348,17 +365,20 @@ they should give the same results as the tests run in TravisCI without special e You can trigger the static checks from the host environment, without entering Docker container. You do that by running appropriate scripts (The same is done in TravisCI) -* [ci_docs.sh](scripts/ci/ci_docs.sh) - checks that documentation can be built without warnings. -* [ci_flake8.sh](scripts/ci/ci_flake8.sh) - runs flake8 source code style guide enforcement tool -* [ci_mypy.sh](scripts/ci/ci_mypy.sh) - runs mypy type annotation consistency check -* [ci_pylint.sh](scripts/ci/ci_pylint.sh) - runs pylint static code checker -* [ci_lint_dockerfile.sh](scripts/ci/ci_lint_dockerfile.sh) - runs lint checker for the Dockerfile -* [ci_check_license.sh](scripts/ci/ci_check_license.sh) - checks if all licences are present in the sources +* [scripts/ci/ci_check_license.sh](scripts/ci/ci_check_license.sh) - checks if all licences are present in the sources +* [scripts/ci/ci_docs.sh](scripts/ci/ci_docs.sh) - checks that documentation can be built without warnings. +* [scripts/ci/ci_flake8.sh](scripts/ci/ci_flake8.sh) - runs flake8 source code style guide enforcement tool +* [scripts/ci/ci_lint_dockerfile.sh](scripts/ci/ci_lint_dockerfile.sh) - runs lint checker for the Dockerfile +* [scripts/ci/ci_mypy.sh](scripts/ci/ci_mypy.sh) - runs mypy type annotation consistency check +* [scripts/ci/ci_pylint_main.sh](scripts/ci/ci_pylint_main.sh) - runs pylint static code checker for main files +* [scripts/ci/ci_pylint_tests.sh](scripts/ci/ci_pylint_tests.sh) - runs pylint static code checker for tests -Those scripts are optimised for time of rebuilds of docker image. The image will be automatically -rebuilt when needed (for example when dependencies change). +Those scripts are optimised for time of rebuilds of docker image. The scripts will fail Review comment: and provide an informative message* 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#discussion_r312721031 ## File path: CONTRIBUTING.md ## @@ -174,7 +184,14 @@ mkvirtualenv --python=python ``` Then you need to install python PIP requirements. Typically it can be done with: -`pip install -e ".[devel]"`. Then you need to run `airflow db init` to create sqlite database. +`pip install -e ".[devel]"`. + +Note - if you have trouble installing mysqlclient on MacOS and you have an error +`ld: library not found for -lssl` - you should run this before running `pip install` command: + +Run: `export LIBRARY_PATH=$LIBRARY_PATH:/usr/local/opt/openssl/lib/` + +After creating the virtualenv you need to run `airflow db init` to create sqlite database. Review comment: After creating the virtualenv, run* 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#discussion_r312721141 ## File path: CONTRIBUTING.md ## @@ -583,67 +611,84 @@ class LoginForm(Form): # pylint: enable=too-few-public-methods ``` -# Git hooks +# Pre-commit hooks -Another great way of automating linting and testing is to use - [Git Hooks](https://git-scm.com/book/uz/v2/Customizing-Git-Git-Hooks). For example you could create a -`pre-commit` file based on the Travis CI Pipeline so that before each commit a local pipeline will be -triggered and if this pipeline fails (returns an exit code other than `0`) the commit does not come through. -This "in theory" has the advantage that you can not commit any code that fails that again reduces the -errors in the Travis CI Pipelines. +Pre-commit hooks are fantastic way of speeding up your local development cycle - you can run exactly the same +static code checks as are run in TravisCI but only limit the checks to the files you are just committing. Review comment: code checks that are running in TravisCI while limiting the checks to the files you are committing.* 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#discussion_r312721179 ## File path: CONTRIBUTING.md ## @@ -583,67 +611,84 @@ class LoginForm(Form): # pylint: enable=too-few-public-methods ``` -# Git hooks +# Pre-commit hooks -Another great way of automating linting and testing is to use - [Git Hooks](https://git-scm.com/book/uz/v2/Customizing-Git-Git-Hooks). For example you could create a -`pre-commit` file based on the Travis CI Pipeline so that before each commit a local pipeline will be -triggered and if this pipeline fails (returns an exit code other than `0`) the commit does not come through. -This "in theory" has the advantage that you can not commit any code that fails that again reduces the -errors in the Travis CI Pipelines. +Pre-commit hooks are fantastic way of speeding up your local development cycle - you can run exactly the same +static code checks as are run in TravisCI but only limit the checks to the files you are just committing. -Since there are a lot of tests the script would last very long so you probably only should test your - new -feature locally. +You are *STRONGLY* encouraged to install pre-commit hooks as they speed up your development and place less +burden on the Travis CI infrastructure. -The following example of a `pre-commit` file allows you.. -- to lint your code via flake8 -- to test your code via nosetests in a docker container based on python 2 -- to test your code via nosetests in a docker container based on python 3 +We have integrated the fantastic [pre-commit](https://pre-commit.com/) framework in our development workflow. +You need to have python 3.6 installed in your host in order to install and use it. It's best to run your +commits when you have your local virtualenv for Airflow activated (then pre-commit and other +dependencies are automatically installed). You can also install pre-commit manually using `pip install`. -``` -#!/bin/sh - -GREEN='\033[0;32m' -NO_COLOR='\033[0m' - -setup_python_env() { -local venv_path=${1} - -echo -e "${GREEN}Activating python virtual environment ${venv_path}..${NO_COLOR}" -source ${venv_path} -} -run_linting() { -local project_dir=$(git rev-parse --show-toplevel) - -echo -e "${GREEN}Running flake8 over directory ${project_dir}..${NO_COLOR}" -flake8 ${project_dir} -} -run_testing_in_docker() { -local feature_path=${1} -local airflow_py2_container=${2} -local airflow_py3_container=${3} - -echo -e "${GREEN}Running tests in ${feature_path} in airflow python 2 docker container..${NO_COLOR}" -docker exec -i -w /airflow/ ${airflow_py2_container} nosetests -v ${feature_path} -echo -e "${GREEN}Running tests in ${feature_path} in airflow python 3 docker container..${NO_COLOR}" -docker exec -i -w /airflow/ ${airflow_py3_container} nosetests -v ${feature_path} -} - -set -e -# NOTE: Before running this make sure you have set the function arguments correctly. -setup_python_env /Users/feluelle/venv/bin/activate -run_linting -run_testing_in_docker tests/contrib/hooks/test_imap_hook.py dazzling_chatterjee quirky_stallman +You need Docker engine configured as the static checks are executed in docker environment. You should +build the images locally before installing pre-commit checks. + +## Installing pre-commit hooks + +`pre-commit install` + +Once you do it - all the commits by default will have to pass the static code analysis checks in order to +be able to add commit. + + +## Docker images for pre-commit hooks + +Before you first install the pre-commit hooks you need to build docker images locally +using `./scripts/ci/local_ci_pull_and_build.sh` to pull images from the repository +or simply `./scripts/ci/local_ci_build.sh` to build the image based on an already pulled image. +Sometimes your image is outdated and needs to be rebuild because some dependencies have been changed. Review comment: be rebuilt* 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#discussion_r312721024 ## File path: CONTRIBUTING.md ## @@ -174,7 +184,14 @@ mkvirtualenv --python=python ``` Then you need to install python PIP requirements. Typically it can be done with: -`pip install -e ".[devel]"`. Then you need to run `airflow db init` to create sqlite database. +`pip install -e ".[devel]"`. + +Note - if you have trouble installing mysqlclient on MacOS and you have an error Review comment: run this command* 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#discussion_r312721172 ## File path: CONTRIBUTING.md ## @@ -583,67 +611,84 @@ class LoginForm(Form): # pylint: enable=too-few-public-methods ``` -# Git hooks +# Pre-commit hooks -Another great way of automating linting and testing is to use - [Git Hooks](https://git-scm.com/book/uz/v2/Customizing-Git-Git-Hooks). For example you could create a -`pre-commit` file based on the Travis CI Pipeline so that before each commit a local pipeline will be -triggered and if this pipeline fails (returns an exit code other than `0`) the commit does not come through. -This "in theory" has the advantage that you can not commit any code that fails that again reduces the -errors in the Travis CI Pipelines. +Pre-commit hooks are fantastic way of speeding up your local development cycle - you can run exactly the same +static code checks as are run in TravisCI but only limit the checks to the files you are just committing. -Since there are a lot of tests the script would last very long so you probably only should test your - new -feature locally. +You are *STRONGLY* encouraged to install pre-commit hooks as they speed up your development and place less +burden on the Travis CI infrastructure. -The following example of a `pre-commit` file allows you.. -- to lint your code via flake8 -- to test your code via nosetests in a docker container based on python 2 -- to test your code via nosetests in a docker container based on python 3 +We have integrated the fantastic [pre-commit](https://pre-commit.com/) framework in our development workflow. +You need to have python 3.6 installed in your host in order to install and use it. It's best to run your +commits when you have your local virtualenv for Airflow activated (then pre-commit and other +dependencies are automatically installed). You can also install pre-commit manually using `pip install`. -``` -#!/bin/sh - -GREEN='\033[0;32m' -NO_COLOR='\033[0m' - -setup_python_env() { -local venv_path=${1} - -echo -e "${GREEN}Activating python virtual environment ${venv_path}..${NO_COLOR}" -source ${venv_path} -} -run_linting() { -local project_dir=$(git rev-parse --show-toplevel) - -echo -e "${GREEN}Running flake8 over directory ${project_dir}..${NO_COLOR}" -flake8 ${project_dir} -} -run_testing_in_docker() { -local feature_path=${1} -local airflow_py2_container=${2} -local airflow_py3_container=${3} - -echo -e "${GREEN}Running tests in ${feature_path} in airflow python 2 docker container..${NO_COLOR}" -docker exec -i -w /airflow/ ${airflow_py2_container} nosetests -v ${feature_path} -echo -e "${GREEN}Running tests in ${feature_path} in airflow python 3 docker container..${NO_COLOR}" -docker exec -i -w /airflow/ ${airflow_py3_container} nosetests -v ${feature_path} -} - -set -e -# NOTE: Before running this make sure you have set the function arguments correctly. -setup_python_env /Users/feluelle/venv/bin/activate -run_linting -run_testing_in_docker tests/contrib/hooks/test_imap_hook.py dazzling_chatterjee quirky_stallman +You need Docker engine configured as the static checks are executed in docker environment. You should +build the images locally before installing pre-commit checks. + +## Installing pre-commit hooks + +`pre-commit install` + +Once you do it - all the commits by default will have to pass the static code analysis checks in order to +be able to add commit. + + +## Docker images for pre-commit hooks + +Before you first install the pre-commit hooks you need to build docker images locally Review comment: before installing the pre-commit hooks you must first build the docker images locally* 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#discussion_r312721158 ## File path: CONTRIBUTING.md ## @@ -583,67 +611,84 @@ class LoginForm(Form): # pylint: enable=too-few-public-methods ``` -# Git hooks +# Pre-commit hooks -Another great way of automating linting and testing is to use - [Git Hooks](https://git-scm.com/book/uz/v2/Customizing-Git-Git-Hooks). For example you could create a -`pre-commit` file based on the Travis CI Pipeline so that before each commit a local pipeline will be -triggered and if this pipeline fails (returns an exit code other than `0`) the commit does not come through. -This "in theory" has the advantage that you can not commit any code that fails that again reduces the -errors in the Travis CI Pipelines. +Pre-commit hooks are fantastic way of speeding up your local development cycle - you can run exactly the same +static code checks as are run in TravisCI but only limit the checks to the files you are just committing. -Since there are a lot of tests the script would last very long so you probably only should test your - new -feature locally. +You are *STRONGLY* encouraged to install pre-commit hooks as they speed up your development and place less +burden on the Travis CI infrastructure. -The following example of a `pre-commit` file allows you.. -- to lint your code via flake8 -- to test your code via nosetests in a docker container based on python 2 -- to test your code via nosetests in a docker container based on python 3 +We have integrated the fantastic [pre-commit](https://pre-commit.com/) framework in our development workflow. +You need to have python 3.6 installed in your host in order to install and use it. It's best to run your +commits when you have your local virtualenv for Airflow activated (then pre-commit and other +dependencies are automatically installed). You can also install pre-commit manually using `pip install`. -``` -#!/bin/sh - -GREEN='\033[0;32m' -NO_COLOR='\033[0m' - -setup_python_env() { -local venv_path=${1} - -echo -e "${GREEN}Activating python virtual environment ${venv_path}..${NO_COLOR}" -source ${venv_path} -} -run_linting() { -local project_dir=$(git rev-parse --show-toplevel) - -echo -e "${GREEN}Running flake8 over directory ${project_dir}..${NO_COLOR}" -flake8 ${project_dir} -} -run_testing_in_docker() { -local feature_path=${1} -local airflow_py2_container=${2} -local airflow_py3_container=${3} - -echo -e "${GREEN}Running tests in ${feature_path} in airflow python 2 docker container..${NO_COLOR}" -docker exec -i -w /airflow/ ${airflow_py2_container} nosetests -v ${feature_path} -echo -e "${GREEN}Running tests in ${feature_path} in airflow python 3 docker container..${NO_COLOR}" -docker exec -i -w /airflow/ ${airflow_py3_container} nosetests -v ${feature_path} -} - -set -e -# NOTE: Before running this make sure you have set the function arguments correctly. -setup_python_env /Users/feluelle/venv/bin/activate -run_linting -run_testing_in_docker tests/contrib/hooks/test_imap_hook.py dazzling_chatterjee quirky_stallman +You need Docker engine configured as the static checks are executed in docker environment. You should Review comment: The pre-commit hooks require Docker Engine to be configured as the static checks* 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#discussion_r312721036 ## File path: CONTRIBUTING.md ## @@ -174,7 +184,14 @@ mkvirtualenv --python=python ``` Then you need to install python PIP requirements. Typically it can be done with: -`pip install -e ".[devel]"`. Then you need to run `airflow db init` to create sqlite database. +`pip install -e ".[devel]"`. + +Note - if you have trouble installing mysqlclient on MacOS and you have an error +`ld: library not found for -lssl` - you should run this before running `pip install` command: + +Run: `export LIBRARY_PATH=$LIBRARY_PATH:/usr/local/opt/openssl/lib/` + +After creating the virtualenv you need to run `airflow db init` to create sqlite database. Review comment: to create a sqlite* 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#discussion_r312721107 ## File path: CONTRIBUTING.md ## @@ -430,6 +450,14 @@ See [Troubleshooting section](#troubleshooting) for steps you can make to clean Once you performed the first build, the images are rebuilt locally rather than pulled - unless you force pull the images. But you can force it using the scripts described below. +## Automated image building + +You can also export one of the three variables to control the behaviour of image rebuilding: + +* Images will be automatically rebuild when needed: `ASSUME_YES="true"` Review comment: Images will automatically rebuild when needed* 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#discussion_r312721073 ## File path: CONTRIBUTING.md ## @@ -348,17 +365,20 @@ they should give the same results as the tests run in TravisCI without special e You can trigger the static checks from the host environment, without entering Docker container. You do that by running appropriate scripts (The same is done in TravisCI) -* [ci_docs.sh](scripts/ci/ci_docs.sh) - checks that documentation can be built without warnings. -* [ci_flake8.sh](scripts/ci/ci_flake8.sh) - runs flake8 source code style guide enforcement tool -* [ci_mypy.sh](scripts/ci/ci_mypy.sh) - runs mypy type annotation consistency check -* [ci_pylint.sh](scripts/ci/ci_pylint.sh) - runs pylint static code checker -* [ci_lint_dockerfile.sh](scripts/ci/ci_lint_dockerfile.sh) - runs lint checker for the Dockerfile -* [ci_check_license.sh](scripts/ci/ci_check_license.sh) - checks if all licences are present in the sources +* [scripts/ci/ci_check_license.sh](scripts/ci/ci_check_license.sh) - checks if all licences are present in the sources +* [scripts/ci/ci_docs.sh](scripts/ci/ci_docs.sh) - checks that documentation can be built without warnings. +* [scripts/ci/ci_flake8.sh](scripts/ci/ci_flake8.sh) - runs flake8 source code style guide enforcement tool +* [scripts/ci/ci_lint_dockerfile.sh](scripts/ci/ci_lint_dockerfile.sh) - runs lint checker for the Dockerfile +* [scripts/ci/ci_mypy.sh](scripts/ci/ci_mypy.sh) - runs mypy type annotation consistency check +* [scripts/ci/ci_pylint_main.sh](scripts/ci/ci_pylint_main.sh) - runs pylint static code checker for main files +* [scripts/ci/ci_pylint_tests.sh](scripts/ci/ci_pylint_tests.sh) - runs pylint static code checker for tests -Those scripts are optimised for time of rebuilds of docker image. The image will be automatically -rebuilt when needed (for example when dependencies change). +Those scripts are optimised for time of rebuilds of docker image. The scripts will fail Review comment: "Those scripts are optimised for time of rebuilds of docker image." Could you please explain this line? I'm not sure I understand 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#discussion_r312721087 ## File path: CONTRIBUTING.md ## @@ -348,17 +365,20 @@ they should give the same results as the tests run in TravisCI without special e You can trigger the static checks from the host environment, without entering Docker container. You do that by running appropriate scripts (The same is done in TravisCI) -* [ci_docs.sh](scripts/ci/ci_docs.sh) - checks that documentation can be built without warnings. -* [ci_flake8.sh](scripts/ci/ci_flake8.sh) - runs flake8 source code style guide enforcement tool -* [ci_mypy.sh](scripts/ci/ci_mypy.sh) - runs mypy type annotation consistency check -* [ci_pylint.sh](scripts/ci/ci_pylint.sh) - runs pylint static code checker -* [ci_lint_dockerfile.sh](scripts/ci/ci_lint_dockerfile.sh) - runs lint checker for the Dockerfile -* [ci_check_license.sh](scripts/ci/ci_check_license.sh) - checks if all licences are present in the sources +* [scripts/ci/ci_check_license.sh](scripts/ci/ci_check_license.sh) - checks if all licences are present in the sources +* [scripts/ci/ci_docs.sh](scripts/ci/ci_docs.sh) - checks that documentation can be built without warnings. +* [scripts/ci/ci_flake8.sh](scripts/ci/ci_flake8.sh) - runs flake8 source code style guide enforcement tool +* [scripts/ci/ci_lint_dockerfile.sh](scripts/ci/ci_lint_dockerfile.sh) - runs lint checker for the Dockerfile +* [scripts/ci/ci_mypy.sh](scripts/ci/ci_mypy.sh) - runs mypy type annotation consistency check +* [scripts/ci/ci_pylint_main.sh](scripts/ci/ci_pylint_main.sh) - runs pylint static code checker for main files +* [scripts/ci/ci_pylint_tests.sh](scripts/ci/ci_pylint_tests.sh) - runs pylint static code checker for tests -Those scripts are optimised for time of rebuilds of docker image. The image will be automatically -rebuilt when needed (for example when dependencies change). +Those scripts are optimised for time of rebuilds of docker image. The scripts will fail +when image rebuild is needed (for example when dependencies change) and provide informative message on +how to rebuild the images. You can control the behaviours as explained in +[Automated image building](#automated-image-building). -You can also force rebuilding of the image by deleting [.build](./build) +You can force rebuilding of the image by deleting [.build](./build) Review comment: You can force an image rebuild* 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#discussion_r312721013 ## File path: CONTRIBUTING.md ## @@ -142,6 +149,9 @@ environment and you can easily debug the code locally. You can also have access contains all the necessary requirements and use it in your local IDE - this aids autocompletion, and running tests directly from within the IDE. +It is **STRONGLY** encouraged to also install and use [Pre commit hooks](#pre-commit-hooks) for your lokal Review comment: local* 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#discussion_r312721183 ## File path: CONTRIBUTING.md ## @@ -583,67 +611,84 @@ class LoginForm(Form): # pylint: enable=too-few-public-methods ``` -# Git hooks +# Pre-commit hooks -Another great way of automating linting and testing is to use - [Git Hooks](https://git-scm.com/book/uz/v2/Customizing-Git-Git-Hooks). For example you could create a -`pre-commit` file based on the Travis CI Pipeline so that before each commit a local pipeline will be -triggered and if this pipeline fails (returns an exit code other than `0`) the commit does not come through. -This "in theory" has the advantage that you can not commit any code that fails that again reduces the -errors in the Travis CI Pipelines. +Pre-commit hooks are fantastic way of speeding up your local development cycle - you can run exactly the same +static code checks as are run in TravisCI but only limit the checks to the files you are just committing. -Since there are a lot of tests the script would last very long so you probably only should test your - new -feature locally. +You are *STRONGLY* encouraged to install pre-commit hooks as they speed up your development and place less +burden on the Travis CI infrastructure. -The following example of a `pre-commit` file allows you.. -- to lint your code via flake8 -- to test your code via nosetests in a docker container based on python 2 -- to test your code via nosetests in a docker container based on python 3 +We have integrated the fantastic [pre-commit](https://pre-commit.com/) framework in our development workflow. +You need to have python 3.6 installed in your host in order to install and use it. It's best to run your +commits when you have your local virtualenv for Airflow activated (then pre-commit and other +dependencies are automatically installed). You can also install pre-commit manually using `pip install`. -``` -#!/bin/sh - -GREEN='\033[0;32m' -NO_COLOR='\033[0m' - -setup_python_env() { -local venv_path=${1} - -echo -e "${GREEN}Activating python virtual environment ${venv_path}..${NO_COLOR}" -source ${venv_path} -} -run_linting() { -local project_dir=$(git rev-parse --show-toplevel) - -echo -e "${GREEN}Running flake8 over directory ${project_dir}..${NO_COLOR}" -flake8 ${project_dir} -} -run_testing_in_docker() { -local feature_path=${1} -local airflow_py2_container=${2} -local airflow_py3_container=${3} - -echo -e "${GREEN}Running tests in ${feature_path} in airflow python 2 docker container..${NO_COLOR}" -docker exec -i -w /airflow/ ${airflow_py2_container} nosetests -v ${feature_path} -echo -e "${GREEN}Running tests in ${feature_path} in airflow python 3 docker container..${NO_COLOR}" -docker exec -i -w /airflow/ ${airflow_py3_container} nosetests -v ${feature_path} -} - -set -e -# NOTE: Before running this make sure you have set the function arguments correctly. -setup_python_env /Users/feluelle/venv/bin/activate -run_linting -run_testing_in_docker tests/contrib/hooks/test_imap_hook.py dazzling_chatterjee quirky_stallman +You need Docker engine configured as the static checks are executed in docker environment. You should +build the images locally before installing pre-commit checks. + +## Installing pre-commit hooks + +`pre-commit install` + +Once you do it - all the commits by default will have to pass the static code analysis checks in order to +be able to add commit. + + +## Docker images for pre-commit hooks + +Before you first install the pre-commit hooks you need to build docker images locally +using `./scripts/ci/local_ci_pull_and_build.sh` to pull images from the repository +or simply `./scripts/ci/local_ci_build.sh` to build the image based on an already pulled image. +Sometimes your image is outdated and needs to be rebuild because some dependencies have been changed. +In such case the docker build pre-commit will fail and inform you that you should perform Review comment: In this case the docker build pre-commit will fail and inform you that you should rebuild the image. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#discussion_r312721167 ## File path: CONTRIBUTING.md ## @@ -583,67 +611,84 @@ class LoginForm(Form): # pylint: enable=too-few-public-methods ``` -# Git hooks +# Pre-commit hooks -Another great way of automating linting and testing is to use - [Git Hooks](https://git-scm.com/book/uz/v2/Customizing-Git-Git-Hooks). For example you could create a -`pre-commit` file based on the Travis CI Pipeline so that before each commit a local pipeline will be -triggered and if this pipeline fails (returns an exit code other than `0`) the commit does not come through. -This "in theory" has the advantage that you can not commit any code that fails that again reduces the -errors in the Travis CI Pipelines. +Pre-commit hooks are fantastic way of speeding up your local development cycle - you can run exactly the same +static code checks as are run in TravisCI but only limit the checks to the files you are just committing. -Since there are a lot of tests the script would last very long so you probably only should test your - new -feature locally. +You are *STRONGLY* encouraged to install pre-commit hooks as they speed up your development and place less +burden on the Travis CI infrastructure. -The following example of a `pre-commit` file allows you.. -- to lint your code via flake8 -- to test your code via nosetests in a docker container based on python 2 -- to test your code via nosetests in a docker container based on python 3 +We have integrated the fantastic [pre-commit](https://pre-commit.com/) framework in our development workflow. +You need to have python 3.6 installed in your host in order to install and use it. It's best to run your +commits when you have your local virtualenv for Airflow activated (then pre-commit and other +dependencies are automatically installed). You can also install pre-commit manually using `pip install`. -``` -#!/bin/sh - -GREEN='\033[0;32m' -NO_COLOR='\033[0m' - -setup_python_env() { -local venv_path=${1} - -echo -e "${GREEN}Activating python virtual environment ${venv_path}..${NO_COLOR}" -source ${venv_path} -} -run_linting() { -local project_dir=$(git rev-parse --show-toplevel) - -echo -e "${GREEN}Running flake8 over directory ${project_dir}..${NO_COLOR}" -flake8 ${project_dir} -} -run_testing_in_docker() { -local feature_path=${1} -local airflow_py2_container=${2} -local airflow_py3_container=${3} - -echo -e "${GREEN}Running tests in ${feature_path} in airflow python 2 docker container..${NO_COLOR}" -docker exec -i -w /airflow/ ${airflow_py2_container} nosetests -v ${feature_path} -echo -e "${GREEN}Running tests in ${feature_path} in airflow python 3 docker container..${NO_COLOR}" -docker exec -i -w /airflow/ ${airflow_py3_container} nosetests -v ${feature_path} -} - -set -e -# NOTE: Before running this make sure you have set the function arguments correctly. -setup_python_env /Users/feluelle/venv/bin/activate -run_linting -run_testing_in_docker tests/contrib/hooks/test_imap_hook.py dazzling_chatterjee quirky_stallman +You need Docker engine configured as the static checks are executed in docker environment. You should +build the images locally before installing pre-commit checks. + +## Installing pre-commit hooks + +`pre-commit install` + +Once you do it - all the commits by default will have to pass the static code analysis checks in order to Review comment: once you do what? I'm not sure what this statement means. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#discussion_r312721101 ## File path: CONTRIBUTING.md ## @@ -430,6 +450,14 @@ See [Troubleshooting section](#troubleshooting) for steps you can make to clean Once you performed the first build, the images are rebuilt locally rather than pulled - unless you force pull the images. But you can force it using the scripts described below. +## Automated image building + +You can also export one of the three variables to control the behaviour of image rebuilding: + +* Images will be automatically rebuild when needed: `ASSUME_YES="true"` Review comment: `ASSUME_YES` is really vague and doesn't tell me what I'm assuming yes to. Could you please change this to something like "ASSUME_AIRFLOW_IMAGE_REBUILD"? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#discussion_r312721122 ## File path: CONTRIBUTING.md ## @@ -430,6 +450,14 @@ See [Troubleshooting section](#troubleshooting) for steps you can make to clean Once you performed the first build, the images are rebuilt locally rather than pulled - unless you force pull the images. But you can force it using the scripts described below. +## Automated image building + +You can also export one of the three variables to control the behaviour of image rebuilding: + +* Images will be automatically rebuild when needed: `ASSUME_YES="true"` +* Old images are used even if rebuild is needed (useful when you have no connectivity): `ASSUME_NO="true"` +* Stop the script if image needs to be rebuild: `ASSUME_QUIT="true"` Review comment: Stops the script if image needs to be rebuilt* 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#discussion_r312721116 ## File path: CONTRIBUTING.md ## @@ -430,6 +450,14 @@ See [Troubleshooting section](#troubleshooting) for steps you can make to clean Once you performed the first build, the images are rebuilt locally rather than pulled - unless you force pull the images. But you can force it using the scripts described below. +## Automated image building + +You can also export one of the three variables to control the behaviour of image rebuilding: + +* Images will be automatically rebuild when needed: `ASSUME_YES="true"` +* Old images are used even if rebuild is needed (useful when you have no connectivity): `ASSUME_NO="true"` Review comment: Will re-use old images even if rebuild is necessary (useful for offline development)* 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] dimberman commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
dimberman commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#issuecomment-520195573 @potiuk dang that was a seriously productive flight. Looking over now :) 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: us...@infra.apache.org With regards, Apache Git Services
[jira] [Commented] (AIRFLOW-2923) LatestOnlyOperator cascade skip through all_done and dummy
[ https://issues.apache.org/jira/browse/AIRFLOW-2923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16904526#comment-16904526 ] ASF GitHub Bot commented on AIRFLOW-2923: - m1racoli commented on pull request #5778: [AIRFLOW-2923][AIRFLOW-1784] Let operators inherit from BaseBranchOperator URL: https://github.com/apache/airflow/pull/5778 Make sure you have checked _all_ steps below. ### Jira - [x] My PR addresses the following [Airflow Jira](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references them in the PR title. For example, "\[AIRFLOW-XXX\] My Airflow PR" - https://issues.apache.org/jira/browse/AIRFLOW-XXX - In case you are fixing a typo in the documentation you can prepend your commit with \[AIRFLOW-XXX\], code changes always need a Jira issue. - In case you are proposing a fundamental code change, you need to create an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)). - In case you are adding a dependency, check if the license complies with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x). ### Description - [x] Here are some details about my PR, including screenshots of any UI changes: LatestOnlyOperator is a special case of a BranchOperator, thus it should inherit from it. This fixes an issue where the skipping behaviour of LatestOnlyOperator is inconsistent with other operators by forcefully skipping all downstream tasks recursively, ignoring trigger rules. In addition BranchPythonOperator should inherit from BaseBranchOperator as well. ### Tests - [x] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason: Extended `tests.operators.test_latest_only_operator.py` to cover downstream children with trigger rules. ### Commits - [x] My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)": 1. Subject is separated from body by a blank line 1. Subject is limited to 50 characters (not including Jira issue reference) 1. Subject does not end with a period 1. Subject uses the imperative mood ("add", not "adding") 1. Body wraps at 72 characters 1. Body explains "what" and "why", not "how" ### Documentation - [x] In case of new functionality, my PR adds documentation that describes how to use it. - All the public functions and the classes in the PR contain docstrings that explain what it does - If you implement backwards incompatible changes, please leave a note in the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so we can assign it to a appropriate release ### Code Quality - [x] Passes `flake8` 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: us...@infra.apache.org > LatestOnlyOperator cascade skip through all_done and dummy > -- > > Key: AIRFLOW-2923 > URL: https://issues.apache.org/jira/browse/AIRFLOW-2923 > Project: Apache Airflow > Issue Type: Bug > Components: scheduler >Affects Versions: 1.9.0 > Environment: CeleryExecutor, 2-nodes cluster, RMQ, PostgreSQL >Reporter: Dmytro Kulyk >Assignee: Cedrik Neumann >Priority: Major > Labels: cascade, latestonly, skip > Attachments: cube_update.py, screenshot-1.png > > > DAG with consolidating point (calc_ready : dummy) > as per [https://airflow.apache.org/concepts.html#latest-run-only] given task > should be ran even catching up an execution DagRuns for a previous periods > However, LatestOnlyOperator cascading through calc_ready despite of it is a > dummy and trigger_rule=all_done > Same behavior when trigger_rule=all_success > {code} > t_ready = DummyOperator( > task_id = 'calc_ready', > trigger_rule = TriggerRule.ALL_DONE, > dag=dag) > {code} > !screenshot-1.png! -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[GitHub] [airflow] m1racoli opened a new pull request #5778: [AIRFLOW-2923][AIRFLOW-1784] Let operators inherit from BaseBranchOperator
m1racoli opened a new pull request #5778: [AIRFLOW-2923][AIRFLOW-1784] Let operators inherit from BaseBranchOperator URL: https://github.com/apache/airflow/pull/5778 Make sure you have checked _all_ steps below. ### Jira - [x] My PR addresses the following [Airflow Jira](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references them in the PR title. For example, "\[AIRFLOW-XXX\] My Airflow PR" - https://issues.apache.org/jira/browse/AIRFLOW-XXX - In case you are fixing a typo in the documentation you can prepend your commit with \[AIRFLOW-XXX\], code changes always need a Jira issue. - In case you are proposing a fundamental code change, you need to create an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)). - In case you are adding a dependency, check if the license complies with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x). ### Description - [x] Here are some details about my PR, including screenshots of any UI changes: LatestOnlyOperator is a special case of a BranchOperator, thus it should inherit from it. This fixes an issue where the skipping behaviour of LatestOnlyOperator is inconsistent with other operators by forcefully skipping all downstream tasks recursively, ignoring trigger rules. In addition BranchPythonOperator should inherit from BaseBranchOperator as well. ### Tests - [x] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason: Extended `tests.operators.test_latest_only_operator.py` to cover downstream children with trigger rules. ### Commits - [x] My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)": 1. Subject is separated from body by a blank line 1. Subject is limited to 50 characters (not including Jira issue reference) 1. Subject does not end with a period 1. Subject uses the imperative mood ("add", not "adding") 1. Body wraps at 72 characters 1. Body explains "what" and "why", not "how" ### Documentation - [x] In case of new functionality, my PR adds documentation that describes how to use it. - All the public functions and the classes in the PR contain docstrings that explain what it does - If you implement backwards incompatible changes, please leave a note in the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so we can assign it to a appropriate release ### Code Quality - [x] Passes `flake8` 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: us...@infra.apache.org With regards, Apache Git Services
[jira] [Assigned] (AIRFLOW-1784) SKIPPED status is being cascading wrongly
[ https://issues.apache.org/jira/browse/AIRFLOW-1784?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Cedrik Neumann reassigned AIRFLOW-1784: --- Assignee: Cedrik Neumann > SKIPPED status is being cascading wrongly > - > > Key: AIRFLOW-1784 > URL: https://issues.apache.org/jira/browse/AIRFLOW-1784 > Project: Apache Airflow > Issue Type: Bug > Components: operators >Affects Versions: 1.8.2 > Environment: Ubuntu 16.04.3 LTS > Python 2.7.12 > CeleryExecutor: 2-nodes cluster >Reporter: Dmytro Kulyk >Assignee: Cedrik Neumann >Priority: Critical > Labels: documentation, latestonly, operators > Attachments: Capture_graph.JPG, Capture_tree2.JPG, cube_update.py > > > After implementation of AIRFLOW-1296 within 1.8.2 there is an wrong behavior > of LatestOnlyOperator which is forcing SKIPPED status cascading despite of > TriggerRule='all_done' set > Which is opposite to documented > [here|https://airflow.incubator.apache.org/concepts.html#latest-run-only] > *Expected Behavior:* > dummy task and all downstreams (update_*) should not be skipped > Full listings are attached > 1.8.1 did not have such issue -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Assigned] (AIRFLOW-2923) LatestOnlyOperator cascade skip through all_done and dummy
[ https://issues.apache.org/jira/browse/AIRFLOW-2923?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Cedrik Neumann reassigned AIRFLOW-2923: --- Assignee: Cedrik Neumann > LatestOnlyOperator cascade skip through all_done and dummy > -- > > Key: AIRFLOW-2923 > URL: https://issues.apache.org/jira/browse/AIRFLOW-2923 > Project: Apache Airflow > Issue Type: Bug > Components: scheduler >Affects Versions: 1.9.0 > Environment: CeleryExecutor, 2-nodes cluster, RMQ, PostgreSQL >Reporter: Dmytro Kulyk >Assignee: Cedrik Neumann >Priority: Major > Labels: cascade, latestonly, skip > Attachments: cube_update.py, screenshot-1.png > > > DAG with consolidating point (calc_ready : dummy) > as per [https://airflow.apache.org/concepts.html#latest-run-only] given task > should be ran even catching up an execution DagRuns for a previous periods > However, LatestOnlyOperator cascading through calc_ready despite of it is a > dummy and trigger_rule=all_done > Same behavior when trigger_rule=all_success > {code} > t_ready = DummyOperator( > task_id = 'calc_ready', > trigger_rule = TriggerRule.ALL_DONE, > dag=dag) > {code} > !screenshot-1.png! -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[GitHub] [airflow] coufon edited a comment on issue #5743: [AIRFLOW-5088][AIP-24] Persisting serialized DAG in DB for webserver scalability
coufon edited a comment on issue #5743: [AIRFLOW-5088][AIP-24] Persisting serialized DAG in DB for webserver scalability URL: https://github.com/apache/airflow/pull/5743#issuecomment-520183227 I saw two remaining issues: (1) Serialized DAGs/Operators are big with many redundant fields. We should trim the fields not used by UI, by giving a list of fields to be included in serialization: class SerializedDAG(DAG, Serialization): _included_fields = list(vars(DAG(dag_id='test')).keys())` class SerializedBaseOperator(BaseOperator, Serialization): _included_fields = list(vars(BaseOperator(task_id='test')).keys()) + [ '_dag', '_task_type', 'subdag', 'ui_color', 'ui_fgcolor', 'template_fields'] Currently we just use all fields of DAG and BaseOperator. (2) (nit) In '/tree' page, it creates a new dagbag instead of using the global dagbag (created once): dag_model = DagModel.get_dagmodel(dag_id) dag = dag_model.get_dag(DAGCACHED_ENABLED) '/graph' page uses the global dagbag. Maybe it is better to unify the behavior. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] coufon edited a comment on issue #5743: [AIRFLOW-5088][AIP-24] Persisting serialized DAG in DB for webserver scalability
coufon edited a comment on issue #5743: [AIRFLOW-5088][AIP-24] Persisting serialized DAG in DB for webserver scalability URL: https://github.com/apache/airflow/pull/5743#issuecomment-520183227 I saw two remaining issues: (1) Serialized DAGs/Operators are big with many redundant fields. We should trim the fields not used by UI, by giving a list of fields to be included in serialization: `class SerializedDAG(DAG, Serialization):\n _included_fields = list(vars(DAG(dag_id='test')).keys())` ` class SerializedBaseOperator(BaseOperator, Serialization): _included_fields = list(vars(BaseOperator(task_id='test')).keys()) + [ '_dag', '_task_type', 'subdag', 'ui_color', 'ui_fgcolor', 'template_fields'] ` Currently we just use all fields of DAG and BaseOperator. (2) (nit) In '/tree' page, it creates a new dagbag instead of using the global dagbag (created once): ` dag_model = DagModel.get_dagmodel(dag_id) dag = dag_model.get_dag(DAGCACHED_ENABLED) ` '/graph' page uses the global dagbag. Maybe it is better to unify the behavior. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] coufon edited a comment on issue #5743: [AIRFLOW-5088][AIP-24] Persisting serialized DAG in DB for webserver scalability
coufon edited a comment on issue #5743: [AIRFLOW-5088][AIP-24] Persisting serialized DAG in DB for webserver scalability URL: https://github.com/apache/airflow/pull/5743#issuecomment-520183227 I saw two remaining issues: (1) Serialized DAGs/Operators are big with many redundant fields. We should trim the fields not used by UI, by giving a list of fields to be included in serialization: `class SerializedDAG(DAG, Serialization): _included_fields = list(vars(DAG(dag_id='test')).keys())` ` ` class SerializedBaseOperator(BaseOperator, Serialization): _included_fields = list(vars(BaseOperator(task_id='test')).keys()) + [ '_dag', '_task_type', 'subdag', 'ui_color', 'ui_fgcolor', 'template_fields'] ` Currently we just use all fields of DAG and BaseOperator. (2) (nit) In '/tree' page, it creates a new dagbag instead of using the global dagbag (created once): ` dag_model = DagModel.get_dagmodel(dag_id) dag = dag_model.get_dag(DAGCACHED_ENABLED) ` '/graph' page uses the global dagbag. Maybe it is better to unify the behavior. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] coufon edited a comment on issue #5743: [AIRFLOW-5088][AIP-24] Persisting serialized DAG in DB for webserver scalability
coufon edited a comment on issue #5743: [AIRFLOW-5088][AIP-24] Persisting serialized DAG in DB for webserver scalability URL: https://github.com/apache/airflow/pull/5743#issuecomment-520183227 I saw two remaining issues: (1) Serialized DAGs/Operators are big with many redundant fields. We should trim the fields not used by UI, by giving a list of fields to be included in serialization: class SerializedDAG(DAG, Serialization): _included_fields = list(vars(DAG(dag_id='test')).keys())` class SerializedBaseOperator(BaseOperator, Serialization): _included_fields = list(vars(BaseOperator(task_id='test')).keys()) + [ '_dag', '_task_type', 'subdag', 'ui_color', 'ui_fgcolor', 'template_fields'] Currently we just use all fields of DAG and BaseOperator. (2) (nit) In '/tree' page, it creates a new dagbag instead of using the global dagbag (created once): dag_model = DagModel.get_dagmodel(dag_id) dag = dag_model.get_dag(DAGCACHED_ENABLED) '/graph' page uses the global dagbag. Maybe it is better to unify the behavior. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] coufon edited a comment on issue #5743: [AIRFLOW-5088][AIP-24] Persisting serialized DAG in DB for webserver scalability
coufon edited a comment on issue #5743: [AIRFLOW-5088][AIP-24] Persisting serialized DAG in DB for webserver scalability URL: https://github.com/apache/airflow/pull/5743#issuecomment-520183227 I saw two remaining issues: (1) Serialized DAGs/Operators are big with many redundant fields. We should trim the fields not used by UI, by giving a list of fields to be included in serialization: `class SerializedDAG(DAG, Serialization): _included_fields = list(vars(DAG(dag_id='test')).keys())` ` class SerializedBaseOperator(BaseOperator, Serialization): _included_fields = list(vars(BaseOperator(task_id='test')).keys()) + [ '_dag', '_task_type', 'subdag', 'ui_color', 'ui_fgcolor', 'template_fields'] ` Currently we just use all fields of DAG and BaseOperator. (2) (nit) In '/tree' page, it creates a new dagbag instead of using the global dagbag (created once): ` dag_model = DagModel.get_dagmodel(dag_id) dag = dag_model.get_dag(DAGCACHED_ENABLED) ` '/graph' page uses the global dagbag. Maybe it is better to unify the behavior. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] coufon commented on issue #5743: [AIRFLOW-5088][AIP-24] Persisting serialized DAG in DB for webserver scalability
coufon commented on issue #5743: [AIRFLOW-5088][AIP-24] Persisting serialized DAG in DB for webserver scalability URL: https://github.com/apache/airflow/pull/5743#issuecomment-520183227 I saw two remaining issues: (1) Serialized DAGs/Operators are big with many redundant fields. We should trim the fields not used by UI, by giving a list of fields to be included in serialization: `class SerializedDAG(DAG, Serialization): _included_fields = list(vars(DAG(dag_id='test')).keys())` ` class SerializedBaseOperator(BaseOperator, Serialization): _included_fields = list(vars(BaseOperator(task_id='test')).keys()) + [ '_dag', '_task_type', 'subdag', 'ui_color', 'ui_fgcolor', 'template_fields'] ` Currently we just use all fields of DAG and BaseOperator. (2) (nit) In '/tree' page, it creates a new dagbag instead of using the global dagbag (created once): ` dag_model = DagModel.get_dagmodel(dag_id) dag = dag_model.get_dag(DAGCACHED_ENABLED) ` '/graph' page uses the global dagbag. Maybe it is better to unify the behavior. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] coufon edited a comment on issue #5743: [AIRFLOW-5088][AIP-24] Persisting serialized DAG in DB for webserver scalability
coufon edited a comment on issue #5743: [AIRFLOW-5088][AIP-24] Persisting serialized DAG in DB for webserver scalability URL: https://github.com/apache/airflow/pull/5743#issuecomment-520183227 I saw two remaining issues: (1) Serialized DAGs/Operators are big with many redundant fields. We should trim the fields not used by UI, by giving a list of fields to be included in serialization: `class SerializedDAG(DAG, Serialization): _included_fields = list(vars(DAG(dag_id='test')).keys())` ` class SerializedBaseOperator(BaseOperator, Serialization): _included_fields = list(vars(BaseOperator(task_id='test')).keys()) + [ '_dag', '_task_type', 'subdag', 'ui_color', 'ui_fgcolor', 'template_fields'] ` Currently we just use all fields of DAG and BaseOperator. (2) (nit) In '/tree' page, it creates a new dagbag instead of using the global dagbag (created once): ` dag_model = DagModel.get_dagmodel(dag_id) dag = dag_model.get_dag(DAGCACHED_ENABLED) ` '/graph' page uses the global dagbag. Maybe it is better to unify the behavior. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] coufon commented on issue #5743: [AIRFLOW-5088][AIP-24] Persisting serialized DAG in DB for webserver scalability
coufon commented on issue #5743: [AIRFLOW-5088][AIP-24] Persisting serialized DAG in DB for webserver scalability URL: https://github.com/apache/airflow/pull/5743#issuecomment-520182466 > Pending Issues: > > * Issue displaying SubDags: This bug is because subdags are not added into the global dagbag in memory when loading from database. There is another bug that self.subdag is a field of SubDagOperator but a field of BaseOperator. So subdag field is not serialized. Added subdag explicitly in serialization. Fixed these two. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] coufon edited a comment on issue #5743: [AIRFLOW-5088][AIP-24] Persisting serialized DAG in DB for webserver scalability
coufon edited a comment on issue #5743: [AIRFLOW-5088][AIP-24] Persisting serialized DAG in DB for webserver scalability URL: https://github.com/apache/airflow/pull/5743#issuecomment-520179192 > Pending Issues: > > * We still have the issue of `SerializedBaseOperator` being displayed in Graph View. This is because graph.html and tree.html use `op.__class__.__name__`. Replaced that by op.task_type to fix 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] coufon commented on issue #5743: [AIRFLOW-5088][AIP-24] Persisting serialized DAG in DB for webserver scalability
coufon commented on issue #5743: [AIRFLOW-5088][AIP-24] Persisting serialized DAG in DB for webserver scalability URL: https://github.com/apache/airflow/pull/5743#issuecomment-520179192 > Pending Issues: > > * We still have the issue of `SerializedBaseOperator` being displayed in Graph View. This is because graph.html and tree.html use op.__class__.__name__. Replaced that by op.task_type to fix 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] ashb commented on issue #5461: [AIRFLOW-4835] Refactor render_template
ashb commented on issue #5461: [AIRFLOW-4835] Refactor render_template URL: https://github.com/apache/airflow/pull/5461#issuecomment-520175353 @bazph I took a week off, will look again in the morning 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
potiuk commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#issuecomment-520168998 All documentation and instructions are in https://github.com/PolideaInternal/airflow/blob/pre-commit-hooks/CONTRIBUTING.md#pre-commit-hooks 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
potiuk commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#issuecomment-520167962 @nuclearpinguin ^^ also you can help to see if that works as expected :) 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
potiuk commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#issuecomment-520167375 Hey. While flying to NYC I managed to rebase, cleanup and trim-down pre-commit hook implementation for all our static checks. I would love if you can take a look, review and try it out. It is built on top of the CI image (and the scripts that build it locally). It is super easy to install and you can use it to run your checks locally. The best thing is that it runs automatically on multiple processors and only for the files you change in your commit - so it is really fast to run pylint + mypy + flake + docker lint (and you do not have to remember which files you should run it on. And it is super easy to trigger it manually. I think if we promote it within community that might significantly improve cycle time for development (especially taking into account TravisCI slowness). The checks are using exactly the same docker image and the same configurations that Travis CI uses - which gives more confidence (not 100% but close) before you push to Travis. I am happy to iterate on it a bit while I am starting holidays as this could be really useful to get better development experience. Once we merge this one I have a number of other pre-commit hooks that we can add over time (validating xml files, yaml, shellcheck. tab removal etc.) - but those can be added gradually - for now I focused on having our current checks in pre-commit. 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: us...@infra.apache.org With regards, Apache Git Services
[jira] [Commented] (AIRFLOW-5161) Add pre-commit hooks to run static checks for only changed files
[ https://issues.apache.org/jira/browse/AIRFLOW-5161?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16904481#comment-16904481 ] ASF GitHub Bot commented on AIRFLOW-5161: - potiuk commented on pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777 Make sure you have checked _all_ steps below. ### Jira - [x] My PR addresses the following [Airflow Jira](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references them in the PR title. For example, "\[AIRFLOW-XXX\] My Airflow PR" - https://issues.apache.org/jira/browse/AIRFLOW-5161 ### Description - [x] Here are some details about my PR, including screenshots of any UI changes: ### Tests - [x] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason: ### Commits - [x] My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)": 1. Subject is separated from body by a blank line 1. Subject is limited to 50 characters (not including Jira issue reference) 1. Subject does not end with a period 1. Subject uses the imperative mood ("add", not "adding") 1. Body wraps at 72 characters 1. Body explains "what" and "why", not "how" ### Documentation - [x] In case of new functionality, my PR adds documentation that describes how to use it. - All the public functions and the classes in the PR contain docstrings that explain what it does - If you implement backwards incompatible changes, please leave a note in the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so we can assign it to a appropriate release ### Code Quality - [x] Passes `flake8` 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: us...@infra.apache.org > Add pre-commit hooks to run static checks for only changed files > > > Key: AIRFLOW-5161 > URL: https://issues.apache.org/jira/browse/AIRFLOW-5161 > Project: Apache Airflow > Issue Type: Improvement > Components: ci >Affects Versions: 2.0.0 >Reporter: Jarek Potiuk >Priority: Major > -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[GitHub] [airflow] potiuk opened a new pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
potiuk opened a new pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777 Make sure you have checked _all_ steps below. ### Jira - [x] My PR addresses the following [Airflow Jira](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references them in the PR title. For example, "\[AIRFLOW-XXX\] My Airflow PR" - https://issues.apache.org/jira/browse/AIRFLOW-5161 ### Description - [x] Here are some details about my PR, including screenshots of any UI changes: ### Tests - [x] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason: ### Commits - [x] My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)": 1. Subject is separated from body by a blank line 1. Subject is limited to 50 characters (not including Jira issue reference) 1. Subject does not end with a period 1. Subject uses the imperative mood ("add", not "adding") 1. Body wraps at 72 characters 1. Body explains "what" and "why", not "how" ### Documentation - [x] In case of new functionality, my PR adds documentation that describes how to use it. - All the public functions and the classes in the PR contain docstrings that explain what it does - If you implement backwards incompatible changes, please leave a note in the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so we can assign it to a appropriate release ### Code Quality - [x] Passes `flake8` 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: us...@infra.apache.org With regards, Apache Git Services
[jira] [Resolved] (AIRFLOW-5159) Optimise checklicence image build (do not build if not needed)
[ https://issues.apache.org/jira/browse/AIRFLOW-5159?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jarek Potiuk resolved AIRFLOW-5159. --- Resolution: Fixed Fix Version/s: 1.10.5 > Optimise checklicence image build (do not build if not needed) > -- > > Key: AIRFLOW-5159 > URL: https://issues.apache.org/jira/browse/AIRFLOW-5159 > Project: Apache Airflow > Issue Type: Improvement > Components: ci >Affects Versions: 2.0.0 >Reporter: Jarek Potiuk >Priority: Major > Fix For: 1.10.5 > > -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (AIRFLOW-5159) Optimise checklicence image build (do not build if not needed)
[ https://issues.apache.org/jira/browse/AIRFLOW-5159?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16904467#comment-16904467 ] ASF subversion and git services commented on AIRFLOW-5159: -- Commit f1d6e7caf79cf414602666403c9dfb0a2a48ece1 in airflow's branch refs/heads/v1-10-test from Jarek Potiuk [ https://gitbox.apache.org/repos/asf?p=airflow.git;h=f1d6e7c ] [AIRFLOW-5159] Checklicence image is not built when not needed (#5774) (cherry picked from commit 8cf063579368a78db960e828fa6ae8973fb640b9) > Optimise checklicence image build (do not build if not needed) > -- > > Key: AIRFLOW-5159 > URL: https://issues.apache.org/jira/browse/AIRFLOW-5159 > Project: Apache Airflow > Issue Type: Improvement > Components: ci >Affects Versions: 2.0.0 >Reporter: Jarek Potiuk >Priority: Major > -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (AIRFLOW-5159) Optimise checklicence image build (do not build if not needed)
[ https://issues.apache.org/jira/browse/AIRFLOW-5159?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16904449#comment-16904449 ] ASF subversion and git services commented on AIRFLOW-5159: -- Commit 8cf063579368a78db960e828fa6ae8973fb640b9 in airflow's branch refs/heads/master from Jarek Potiuk [ https://gitbox.apache.org/repos/asf?p=airflow.git;h=8cf0635 ] [AIRFLOW-5159] Checklicence image is not built when not needed (#5774) > Optimise checklicence image build (do not build if not needed) > -- > > Key: AIRFLOW-5159 > URL: https://issues.apache.org/jira/browse/AIRFLOW-5159 > Project: Apache Airflow > Issue Type: Improvement > Components: ci >Affects Versions: 2.0.0 >Reporter: Jarek Potiuk >Priority: Major > -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (AIRFLOW-5159) Optimise checklicence image build (do not build if not needed)
[ https://issues.apache.org/jira/browse/AIRFLOW-5159?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16904447#comment-16904447 ] ASF GitHub Bot commented on AIRFLOW-5159: - potiuk commented on pull request #5774: [AIRFLOW-5159] Checklicence image is not built when not needed URL: https://github.com/apache/airflow/pull/5774 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: us...@infra.apache.org > Optimise checklicence image build (do not build if not needed) > -- > > Key: AIRFLOW-5159 > URL: https://issues.apache.org/jira/browse/AIRFLOW-5159 > Project: Apache Airflow > Issue Type: Improvement > Components: ci >Affects Versions: 2.0.0 >Reporter: Jarek Potiuk >Priority: Major > -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[GitHub] [airflow] potiuk merged pull request #5774: [AIRFLOW-5159] Checklicence image is not built when not needed
potiuk merged pull request #5774: [AIRFLOW-5159] Checklicence image is not built when not needed URL: https://github.com/apache/airflow/pull/5774 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: us...@infra.apache.org With regards, Apache Git Services
[jira] [Created] (AIRFLOW-5161) Add pre-commit hooks to run static checks for only changed files
Jarek Potiuk created AIRFLOW-5161: - Summary: Add pre-commit hooks to run static checks for only changed files Key: AIRFLOW-5161 URL: https://issues.apache.org/jira/browse/AIRFLOW-5161 Project: Apache Airflow Issue Type: Improvement Components: ci Affects Versions: 2.0.0 Reporter: Jarek Potiuk -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[GitHub] [airflow] mik-laj opened a new pull request #5776: [AIRFLOW-XXX] Add section in TOC with all reference guides
mik-laj opened a new pull request #5776: [AIRFLOW-XXX] Add section in TOC with all reference guides URL: https://github.com/apache/airflow/pull/5776 Make sure you have checked _all_ steps below. ### Jira - [ ] My PR addresses the following [Airflow Jira](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references them in the PR title. For example, "\[AIRFLOW-XXX\] My Airflow PR" - https://issues.apache.org/jira/browse/AIRFLOW-XXX - In case you are fixing a typo in the documentation you can prepend your commit with \[AIRFLOW-XXX\], code changes always need a Jira issue. - In case you are proposing a fundamental code change, you need to create an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)). - In case you are adding a dependency, check if the license complies with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x). ### Description - [ ] Here are some details about my PR, including screenshots of any UI changes: ### Tests - [ ] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason: ### Commits - [ ] My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)": 1. Subject is separated from body by a blank line 1. Subject is limited to 50 characters (not including Jira issue reference) 1. Subject does not end with a period 1. Subject uses the imperative mood ("add", not "adding") 1. Body wraps at 72 characters 1. Body explains "what" and "why", not "how" ### Documentation - [ ] In case of new functionality, my PR adds documentation that describes how to use it. - All the public functions and the classes in the PR contain docstrings that explain what it does - If you implement backwards incompatible changes, please leave a note in the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so we can assign it to a appropriate release ### Code Quality - [ ] Passes `flake8` 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on issue #5774: [AIRFLOW-5159] Checklicence image is not built when not needed
potiuk commented on issue #5774: [AIRFLOW-5159] Checklicence image is not built when not needed URL: https://github.com/apache/airflow/pull/5774#issuecomment-520145968 This one will be helpful to get pre-commits working (follow-up 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Commented] (AIRFLOW-5160) Remove DAG count test
[ https://issues.apache.org/jira/browse/AIRFLOW-5160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16904409#comment-16904409 ] ASF GitHub Bot commented on AIRFLOW-5160: - BasPH commented on pull request #5775: [AIRFLOW-5160] Remove example DAG count test URL: https://github.com/apache/airflow/pull/5775 Make sure you have checked _all_ steps below. ### Jira - [x] My PR addresses the following [Airflow Jira](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references them in the PR title. For example, "\[AIRFLOW-XXX\] My Airflow PR" - https://issues.apache.org/jira/browse/AIRFLOW-5160 - In case you are fixing a typo in the documentation you can prepend your commit with \[AIRFLOW-XXX\], code changes always need a Jira issue. - In case you are proposing a fundamental code change, you need to create an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)). - In case you are adding a dependency, check if the license complies with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x). ### Description - [x] Here are some details about my PR, including screenshots of any UI changes: ### Tests - [x] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason: The test_import_examples test asserts the (hardcoded) number of example DAGs. It does not verify any behaviour and fails when example DAGs are added/removed. It doesn't test behaviour so I suggest to remove it. ### Commits - [ ] My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)": 1. Subject is separated from body by a blank line 1. Subject is limited to 50 characters (not including Jira issue reference) 1. Subject does not end with a period 1. Subject uses the imperative mood ("add", not "adding") 1. Body wraps at 72 characters 1. Body explains "what" and "why", not "how" ### Documentation - [ ] In case of new functionality, my PR adds documentation that describes how to use it. - All the public functions and the classes in the PR contain docstrings that explain what it does - If you implement backwards incompatible changes, please leave a note in the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so we can assign it to a appropriate release ### Code Quality - [ ] Passes `flake8` 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: us...@infra.apache.org > Remove DAG count test > - > > Key: AIRFLOW-5160 > URL: https://issues.apache.org/jira/browse/AIRFLOW-5160 > Project: Apache Airflow > Issue Type: Improvement > Components: tests >Affects Versions: 2.0.0, 1.10.5 >Reporter: Bas Harenslak >Priority: Major > > The test_import_examples test asserts the number of example DAGs. It does not > verify any behaviour and fails when example DAGs are added/removed. It > doesn't test behaviour so I suggest to remove it. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[GitHub] [airflow] BasPH opened a new pull request #5775: [AIRFLOW-5160] Remove example DAG count test
BasPH opened a new pull request #5775: [AIRFLOW-5160] Remove example DAG count test URL: https://github.com/apache/airflow/pull/5775 Make sure you have checked _all_ steps below. ### Jira - [x] My PR addresses the following [Airflow Jira](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references them in the PR title. For example, "\[AIRFLOW-XXX\] My Airflow PR" - https://issues.apache.org/jira/browse/AIRFLOW-5160 - In case you are fixing a typo in the documentation you can prepend your commit with \[AIRFLOW-XXX\], code changes always need a Jira issue. - In case you are proposing a fundamental code change, you need to create an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)). - In case you are adding a dependency, check if the license complies with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x). ### Description - [x] Here are some details about my PR, including screenshots of any UI changes: ### Tests - [x] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason: The test_import_examples test asserts the (hardcoded) number of example DAGs. It does not verify any behaviour and fails when example DAGs are added/removed. It doesn't test behaviour so I suggest to remove it. ### Commits - [ ] My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)": 1. Subject is separated from body by a blank line 1. Subject is limited to 50 characters (not including Jira issue reference) 1. Subject does not end with a period 1. Subject uses the imperative mood ("add", not "adding") 1. Body wraps at 72 characters 1. Body explains "what" and "why", not "how" ### Documentation - [ ] In case of new functionality, my PR adds documentation that describes how to use it. - All the public functions and the classes in the PR contain docstrings that explain what it does - If you implement backwards incompatible changes, please leave a note in the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so we can assign it to a appropriate release ### Code Quality - [ ] Passes `flake8` 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] BasPH commented on issue #5461: [AIRFLOW-4835] Refactor render_template
BasPH commented on issue #5461: [AIRFLOW-4835] Refactor render_template URL: https://github.com/apache/airflow/pull/5461#issuecomment-520141652 @ashb any other remarks on this PR, or is it good to go? 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: us...@infra.apache.org With regards, Apache Git Services
[jira] [Created] (AIRFLOW-5160) Remove DAG count test
Bas Harenslak created AIRFLOW-5160: -- Summary: Remove DAG count test Key: AIRFLOW-5160 URL: https://issues.apache.org/jira/browse/AIRFLOW-5160 Project: Apache Airflow Issue Type: Improvement Components: tests Affects Versions: 2.0.0, 1.10.5 Reporter: Bas Harenslak The test_import_examples test asserts the number of example DAGs. It does not verify any behaviour and fails when example DAGs are added/removed. It doesn't test behaviour so I suggest to remove it. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (AIRFLOW-5159) Optimise checklicence image build (do not build if not needed)
[ https://issues.apache.org/jira/browse/AIRFLOW-5159?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16904355#comment-16904355 ] ASF GitHub Bot commented on AIRFLOW-5159: - potiuk commented on pull request #5774: [AIRFLOW-5159] Checklicence image is not built when not needed URL: https://github.com/apache/airflow/pull/5774 Make sure you have checked _all_ steps below. ### Jira - [x] My PR addresses the following [Airflow Jira](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references them in the PR title. For example, "\[AIRFLOW-XXX\] My Airflow PR" - https://issues.apache.org/jira/browse/AIRFLOW-5159 ### Description - [x] Here are some details about my PR, including screenshots of any UI changes: ### Tests - [x] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason: ### Commits - [x] My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)": 1. Subject is separated from body by a blank line 1. Subject is limited to 50 characters (not including Jira issue reference) 1. Subject does not end with a period 1. Subject uses the imperative mood ("add", not "adding") 1. Body wraps at 72 characters 1. Body explains "what" and "why", not "how" ### Documentation - [x] In case of new functionality, my PR adds documentation that describes how to use it. - All the public functions and the classes in the PR contain docstrings that explain what it does - If you implement backwards incompatible changes, please leave a note in the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so we can assign it to a appropriate release ### Code Quality - [x] Passes `flake8` 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: us...@infra.apache.org > Optimise checklicence image build (do not build if not needed) > -- > > Key: AIRFLOW-5159 > URL: https://issues.apache.org/jira/browse/AIRFLOW-5159 > Project: Apache Airflow > Issue Type: Improvement > Components: ci >Affects Versions: 2.0.0 >Reporter: Jarek Potiuk >Priority: Major > -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[GitHub] [airflow] potiuk commented on issue #5773: [AIRFLOW-XXX] Adding AloPeyk to the list of Airflow users
potiuk commented on issue #5773: [AIRFLOW-XXX] Adding AloPeyk to the list of Airflow users URL: https://github.com/apache/airflow/pull/5773#issuecomment-520124304 Welcome! 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on issue #5772: [AIRFLOW-XXX] Add iS2.co to list of airflow users
potiuk commented on issue #5772: [AIRFLOW-XXX] Add iS2.co to list of airflow users URL: https://github.com/apache/airflow/pull/5772#issuecomment-520124348 Welcome! 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk merged pull request #5772: [AIRFLOW-XXX] Add iS2.co to list of airflow users
potiuk merged pull request #5772: [AIRFLOW-XXX] Add iS2.co to list of airflow users URL: https://github.com/apache/airflow/pull/5772 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk merged pull request #5773: [AIRFLOW-XXX] Adding AloPeyk to the list of Airflow users
potiuk merged pull request #5773: [AIRFLOW-XXX] Adding AloPeyk to the list of Airflow users URL: https://github.com/apache/airflow/pull/5773 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk opened a new pull request #5774: [AIRFLOW-5159] Checklicence image is not built when not needed
potiuk opened a new pull request #5774: [AIRFLOW-5159] Checklicence image is not built when not needed URL: https://github.com/apache/airflow/pull/5774 Make sure you have checked _all_ steps below. ### Jira - [x] My PR addresses the following [Airflow Jira](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references them in the PR title. For example, "\[AIRFLOW-XXX\] My Airflow PR" - https://issues.apache.org/jira/browse/AIRFLOW-5159 ### Description - [x] Here are some details about my PR, including screenshots of any UI changes: ### Tests - [x] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason: ### Commits - [x] My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)": 1. Subject is separated from body by a blank line 1. Subject is limited to 50 characters (not including Jira issue reference) 1. Subject does not end with a period 1. Subject uses the imperative mood ("add", not "adding") 1. Body wraps at 72 characters 1. Body explains "what" and "why", not "how" ### Documentation - [x] In case of new functionality, my PR adds documentation that describes how to use it. - All the public functions and the classes in the PR contain docstrings that explain what it does - If you implement backwards incompatible changes, please leave a note in the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so we can assign it to a appropriate release ### Code Quality - [x] Passes `flake8` 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: us...@infra.apache.org With regards, Apache Git Services
[jira] [Created] (AIRFLOW-5159) Optimise checklicence image build (do not build if not needed)
Jarek Potiuk created AIRFLOW-5159: - Summary: Optimise checklicence image build (do not build if not needed) Key: AIRFLOW-5159 URL: https://issues.apache.org/jira/browse/AIRFLOW-5159 Project: Apache Airflow Issue Type: Improvement Components: ci Affects Versions: 2.0.0 Reporter: Jarek Potiuk -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[GitHub] [airflow] bijanmoudi opened a new pull request #5773: [AIRFLOW-XXX] Adding AloPeyk to the list of Airflow users
bijanmoudi opened a new pull request #5773: [AIRFLOW-XXX] Adding AloPeyk to the list of Airflow users URL: https://github.com/apache/airflow/pull/5773 Make sure you have checked _all_ steps below. ### Jira - [ ] My PR addresses the following [Airflow Jira](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references them in the PR title. For example, "\[AIRFLOW-XXX\] My Airflow PR" - https://issues.apache.org/jira/browse/AIRFLOW-XXX - In case you are fixing a typo in the documentation you can prepend your commit with \[AIRFLOW-XXX\], code changes always need a Jira issue. - In case you are proposing a fundamental code change, you need to create an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)). - In case you are adding a dependency, check if the license complies with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x). ### Description - [ ] Here are some details about my PR, including screenshots of any UI changes: ### Tests - [ ] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason: ### Commits - [ ] My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)": 1. Subject is separated from body by a blank line 1. Subject is limited to 50 characters (not including Jira issue reference) 1. Subject does not end with a period 1. Subject uses the imperative mood ("add", not "adding") 1. Body wraps at 72 characters 1. Body explains "what" and "why", not "how" ### Documentation - [ ] In case of new functionality, my PR adds documentation that describes how to use it. - All the public functions and the classes in the PR contain docstrings that explain what it does - If you implement backwards incompatible changes, please leave a note in the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so we can assign it to a appropriate release ### Code Quality - [ ] Passes `flake8` 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: us...@infra.apache.org With regards, Apache Git Services