You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/08/18 22:40:54 UTC
[GitHub] [airflow] potiuk opened a new pull request #10390: When precommits are run, output is silenced
potiuk opened a new pull request #10390:
URL: https://github.com/apache/airflow/pull/10390
The output of pre-commit builds on both CI and locally
is now limited to only show errors, unless verbose
variable is set.
We are utilising aliases if possible but in case of
pre-commits they are run in non-interactive shell which
means that aliases do not work as expected so we have
to run a few functions directly in other to
show spinner.
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #10390: When precommits are run, output is silenced
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10390:
URL: https://github.com/apache/airflow/pull/10390#discussion_r472590273
##########
File path: scripts/ci/libraries/_build_images.sh
##########
@@ -559,12 +558,27 @@ function build_ci_image() {
echo >&2
exit 1
fi
+ if [[ -n ${SPIN_PID:=""} ]]; then
+ kill -HUP "${SPIN_PID}" || true
+ wait "${SPIN_PID}" || true
+ echo > "${DETECTED_TERMINAL}"
Review comment:
What values can the variable ``DETECTED_TERMINAL`` have? I can see that this variable is always set to `/dev/tty`. Did I miss something?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #10390: When precommits are run, output is silenced
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #10390:
URL: https://github.com/apache/airflow/pull/10390
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #10390: When precommits are run, output is silenced
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #10390:
URL: https://github.com/apache/airflow/pull/10390#issuecomment-675759812
@mik-laj -> this one nicely fixes the output of pre-commits locally and GitHub actions so that no extra stuff is visible. Still it keeps pre-commit spinner /progress running as well as handles well VERBOSE flag. And I switched to using aliases for docker/kubectl everywhere where I can (aliases do not work in pre-commits as they are not interactive)
Of course erors are printed on 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:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #10390: When precommits are run, output is silenced
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #10390:
URL: https://github.com/apache/airflow/pull/10390#issuecomment-675759812
@mik-laj -> this one nicely fixes the output of pre-commits locally and GitHub actions so that no extra stuff is visible. Still it keeps pre-commit spinner /progress running as well as handles well VERBOSE flag. And I switched to using aliases for docker/kubectl everywhere where I can (aliases do not work in pre-commits as they are not interactive)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #10390: When precommits are run, output is silenced
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #10390:
URL: https://github.com/apache/airflow/pull/10390#discussion_r472785575
##########
File path: scripts/ci/libraries/_build_images.sh
##########
@@ -559,12 +558,27 @@ function build_ci_image() {
echo >&2
exit 1
fi
+ if [[ -n ${SPIN_PID:=""} ]]; then
+ kill -HUP "${SPIN_PID}" || true
+ wait "${SPIN_PID}" || true
+ echo > "${DETECTED_TERMINAL}"
Review comment:
it can be /dev/tty if we cannot detect therminal with $(tty) (which happens inside pre-commit on git commit (it is always non-interactive by design)
##########
File path: scripts/ci/libraries/_build_images.sh
##########
@@ -559,12 +558,27 @@ function build_ci_image() {
echo >&2
exit 1
fi
+ if [[ -n ${SPIN_PID:=""} ]]; then
+ kill -HUP "${SPIN_PID}" || true
+ wait "${SPIN_PID}" || true
+ echo > "${DETECTED_TERMINAL}"
Review comment:
it can be /dev/tty if we cannot detect therminal with $(tty) (which happens inside pre-commit on git commit (it is always non-interactive by design)
```
elif [[ ${DETECTED_TERMINAL:=$(tty)} != "not a tty" ]]; then
````
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #10390: When precommits are run, output is silenced
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #10390:
URL: https://github.com/apache/airflow/pull/10390#discussion_r472800865
##########
File path: scripts/ci/libraries/_build_images.sh
##########
@@ -559,12 +558,27 @@ function build_ci_image() {
echo >&2
exit 1
fi
+ if [[ -n ${SPIN_PID:=""} ]]; then
+ kill -HUP "${SPIN_PID}" || true
+ wait "${SPIN_PID}" || true
+ echo > "${DETECTED_TERMINAL}"
Review comment:
The whole thing is about trying to print progress to most likely terminal i fbuild happens insde pre-commit inside `git commit command - it won't always succeed but in vast majority of cases of developer workstation - it will
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org