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/14 14:58:04 UTC
[GitHub] [airflow] mik-laj opened a new pull request #10329: More fancy environment checking
mik-laj opened a new pull request #10329:
URL: https://github.com/apache/airflow/pull/10329
<img width="721" alt="Screenshot 2020-08-14 at 16 45 42" src="https://user-images.githubusercontent.com/12058428/90262948-43d47080-de4f-11ea-9668-9975753700b8.png">
<img width="685" alt="Screenshot 2020-08-14 at 16 52 46" src="https://user-images.githubusercontent.com/12058428/90262951-45059d80-de4f-11ea-9ccd-0a1d1eef016b.png">
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/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] potiuk commented on a change in pull request #10329: Breeze: More fancy environment checking
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #10329:
URL: https://github.com/apache/airflow/pull/10329#discussion_r471025217
##########
File path: scripts/ci/in_container/check_environment.sh
##########
@@ -131,34 +119,21 @@ function resetdb_if_requested() {
return $?
}
-if [[ -n ${BACKEND:=} ]]; then
- echo "==============================================================================================="
- echo " Checking backend: ${BACKEND}"
- echo "==============================================================================================="
- set +e
+echo "==============================================================================================="
+echo " Checking integrations and backends"
+echo "==============================================================================================="
+if [[ -n ${BACKEND:=} ]]; then
Review comment:
The problem is that you can get some extra output in this check. Bash function might either pass output by printing result in the output or returning exit code. I. This case I chose exit code in other to avoid ambiguity of printed stdout by the check command. It is of course possible to avoid it but I figured exit code is better, simpler and avoids unexpected behaviour.
But feel free to change it if you take care about the ambiguity .
----------------------------------------------------------------
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 #10329: More fancy environment checking
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10329:
URL: https://github.com/apache/airflow/pull/10329#discussion_r470920057
##########
File path: scripts/ci/in_container/check_environment.sh
##########
@@ -131,34 +119,21 @@ function resetdb_if_requested() {
return $?
}
-if [[ -n ${BACKEND:=} ]]; then
- echo "==============================================================================================="
- echo " Checking backend: ${BACKEND}"
- echo "==============================================================================================="
- set +e
+echo "==============================================================================================="
+echo " Checking integrations and backends"
+echo "==============================================================================================="
+if [[ -n ${BACKEND:=} ]]; then
Review comment:
I don't think it is like you say. This function communicates through a global variable - EXIT_CODE, not "real" exit codes. We can clean up some code in this file to make it clearer. Thanks for comment.
----------------------------------------------------------------
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 #10329: More fancy environment checking
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10329:
URL: https://github.com/apache/airflow/pull/10329#discussion_r470921708
##########
File path: scripts/ci/in_container/check_environment.sh
##########
@@ -23,142 +23,95 @@ EXIT_CODE=0
DISABLED_INTEGRATIONS=""
-function check_integration {
+function check_service {
INTEGRATION_NAME=$1
CALL=$2
MAX_CHECK=${3:=1}
- ENV_VAR_NAME=INTEGRATION_${INTEGRATION_NAME^^}
- if [[ ${!ENV_VAR_NAME:=} != "true" ]]; then
- DISABLED_INTEGRATIONS="${DISABLED_INTEGRATIONS} ${INTEGRATION_NAME}"
- return
- fi
-
- echo "-----------------------------------------------------------------------------------------------"
- echo " Checking integration ${INTEGRATION_NAME}"
- echo "-----------------------------------------------------------------------------------------------"
+ echo -n "${INTEGRATION_NAME}: "
while true
do
set +e
LAST_CHECK_RESULT=$(eval "${CALL}" 2>&1)
RES=$?
set -e
if [[ ${RES} == 0 ]]; then
- echo
- echo " Integration ${INTEGRATION_NAME} OK!"
- echo
+ echo -e " \e[32mOK.\e[0m"
break
else
echo -n "."
MAX_CHECK=$((MAX_CHECK-1))
fi
if [[ ${MAX_CHECK} == 0 ]]; then
- echo
- echo "ERROR! Maximum number of retries while checking ${INTEGRATION_NAME} integration. Exiting"
- echo
+ echo -e " \e[31mERROR!\e[30m"
+ echo "Maximum number of retries while checking service. Exiting"
break
else
sleep 1
fi
done
if [[ ${RES} != 0 ]]; then
- echo " ERROR: Integration ${INTEGRATION_NAME} could not be started!"
+ echo "Service could not be started!"
echo
echo "${LAST_CHECK_RESULT}"
echo
EXIT_CODE=${RES}
fi
- echo "-----------------------------------------------------------------------------------------------"
}
-function check_db_connection {
- MAX_CHECK=${1:=3}
+function check_integration {
+ INTEGRATION_NAME=$1
+
+ ENV_VAR_NAME=INTEGRATION_${INTEGRATION_NAME^^}
+ if [[ ${!ENV_VAR_NAME:=} != "true" ]]; then
+ DISABLED_INTEGRATIONS="${DISABLED_INTEGRATIONS} ${INTEGRATION_NAME}"
+ return
+ fi
+ check_service "${@}"
+}
+
+function check_db_backend {
+ MAX_CHECK=${1:=1}
if [[ ${BACKEND} == "postgres" ]]; then
- HOSTNAME=postgres
- PORT=5432
+ check_service "postgres" "nc -zvv postgres 5432" "${MAX_CHECK}"
elif [[ ${BACKEND} == "mysql" ]]; then
- HOSTNAME=mysql
- PORT=3306
- else
+ check_service "mysql" "nc -zvv mysql 3306" "${MAX_CHECK}"
+ elif [[ ${BACKEND} == "sqlite" ]]; then
return
+ else
+ echo "Unknown backend. Supported values: [postgres,mysql,sqlite]. Current value: [${BACKEND}]"
+ exit 1
fi
- echo "-----------------------------------------------------------------------------------------------"
- echo " Checking DB ${BACKEND}"
- echo "-----------------------------------------------------------------------------------------------"
- while true
- do
- set +e
- LAST_CHECK_RESULT=$(nc -zvv ${HOSTNAME} ${PORT} 2>&1)
- RES=$?
- set -e
- if [[ ${RES} == 0 ]]; then
- echo
- echo " Backend ${BACKEND} OK!"
- echo
- break
- else
- echo -n "."
- MAX_CHECK=$((MAX_CHECK-1))
- fi
- if [[ ${MAX_CHECK} == 0 ]]; then
- echo
- echo "ERROR! Maximum number of retries while checking ${BACKEND} db. Exiting"
- echo
- break
- else
- sleep 1
- fi
- done
- if [[ ${RES} != 0 ]]; then
- echo " ERROR: ${BACKEND} db could not be reached!"
- echo
- echo "${LAST_CHECK_RESULT}"
- echo
- EXIT_CODE=${RES}
- fi
- echo "-----------------------------------------------------------------------------------------------"
}
function resetdb_if_requested() {
if [[ ${DB_RESET:="false"} == "true" ]]; then
if [[ ${RUN_AIRFLOW_1_10} == "true" ]]; then
- airflow resetdb -y
+ airflow resetdb -y
else
- airflow db reset -y
+ airflow db reset -y
fi
fi
return $?
}
-if [[ -n ${BACKEND:=} ]]; then
- echo "==============================================================================================="
- echo " Checking backend: ${BACKEND}"
- echo "==============================================================================================="
-
- set +e
- check_db_connection 20
- set -e
- if [[ ${EXIT_CODE} == 0 ]]; then
- echo "==============================================================================================="
- echo " Backend database is sane"
- echo "==============================================================================================="
- echo
- fi
-else
- echo "==============================================================================================="
- echo " Skip checking backend - BACKEND not set"
- echo "==============================================================================================="
- echo
+echo "==============================================================================================="
+echo " Checking integrations and backends"
+echo "==============================================================================================="
+if [[ -n ${BACKEND:=} ]]; then
+ check_db_backend 20
+ echo "-----------------------------------------------------------------------------------------------"
fi
-
check_integration kerberos "nc -zvv kerberos 88" 30
check_integration mongo "nc -zvv mongo 27017" 20
check_integration redis "nc -zvv redis 6379" 20
check_integration rabbitmq "nc -zvv rabbitmq 5672" 20
check_integration cassandra "nc -zvv cassandra 9042" 20
check_integration openldap "nc -zvv openldap 389" 20
+check_integration presto "nc -zvv presto 8080" 40
Review comment:
Presto is a very slow service. For me, it takes 25 dots to start.
----------------------------------------------------------------
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 #10329: Breeze: More fancy environment checking
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #10329:
URL: https://github.com/apache/airflow/pull/10329
----------------------------------------------------------------
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 pull request #10329: Breeze: More fancy environment checking
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #10329:
URL: https://github.com/apache/airflow/pull/10329#issuecomment-674430564
@potiuk All green.
----------------------------------------------------------------
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 #10329: More fancy environment checking
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #10329:
URL: https://github.com/apache/airflow/pull/10329#issuecomment-674125974
Though there are some errors :)
----------------------------------------------------------------
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 #10329: More fancy environment checking
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #10329:
URL: https://github.com/apache/airflow/pull/10329#discussion_r470692592
##########
File path: scripts/ci/in_container/check_environment.sh
##########
@@ -131,34 +119,21 @@ function resetdb_if_requested() {
return $?
}
-if [[ -n ${BACKEND:=} ]]; then
- echo "==============================================================================================="
- echo " Checking backend: ${BACKEND}"
- echo "==============================================================================================="
- set +e
+echo "==============================================================================================="
+echo " Checking integrations and backends"
+echo "==============================================================================================="
+if [[ -n ${BACKEND:=} ]]; then
Review comment:
set +e / -e should be around that - check_db_connection exits with '1" in case of error.
----------------------------------------------------------------
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 #10329: More fancy environment checking
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10329:
URL: https://github.com/apache/airflow/pull/10329#discussion_r470920057
##########
File path: scripts/ci/in_container/check_environment.sh
##########
@@ -131,34 +119,21 @@ function resetdb_if_requested() {
return $?
}
-if [[ -n ${BACKEND:=} ]]; then
- echo "==============================================================================================="
- echo " Checking backend: ${BACKEND}"
- echo "==============================================================================================="
- set +e
+echo "==============================================================================================="
+echo " Checking integrations and backends"
+echo "==============================================================================================="
+if [[ -n ${BACKEND:=} ]]; then
Review comment:
It's strange that one function behaves differently from the other. We can clean up some code in this file to make it clearer. Thanks for comment.
----------------------------------------------------------------
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