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/04 12:00:43 UTC
[GitHub] [airflow] potiuk opened a new pull request #10158: Documentation artifact are also uploaded as GitHub Actions Artifacts
potiuk opened a new pull request #10158:
URL: https://github.com/apache/airflow/pull/10158
---
**^ 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 #10158: Documentation artifact are also uploaded as GitHub Actions Artifacts
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #10158:
URL: https://github.com/apache/airflow/pull/10158#discussion_r465632652
##########
File path: scripts/ci/libraries/_initialization.sh
##########
@@ -131,7 +131,7 @@ function initialize_common_environment {
print_info
print_info "Mount whole airflow source directory for static checks"
print_info
- EXTRA_DOCKER_FLAGS=( \
+ EXTRA_DOCKER_FLAGS=(
"-v" "${AIRFLOW_SOURCES}:/opt/airflow" \
Review comment:
Yep
----------------------------------------------------------------
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] feluelle commented on a change in pull request #10158: Documentation artifact are also uploaded as GitHub Actions Artifacts
Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #10158:
URL: https://github.com/apache/airflow/pull/10158#discussion_r465103961
##########
File path: scripts/ci/libraries/_initialization.sh
##########
@@ -148,6 +151,23 @@ function initialize_common_environment {
)
fi
+ # Common variables passed via docker commands
+ EXTRA_DOCKER_FLAGS+=(
+ "--env" "INSTALL_AIRFLOW_VERSION"
+ "--env" "PYTHONDONTWRITEBYTECODE"
+ "--env" "VERBOSE"
+ "--env" "VERBOSE_COMMANDS"
+ "--env" "HOST_USER_ID"
+ "--env" "HOST_GROUP_ID"
+ "--env" "HOST_OS"
+ "--env" "HOST_HOME"
+ "--env" "HOST_AIRFLOW_SOURCES"
+ "--env" "PYTHON_MAJOR_MINOR_VERSION"
+ "--env" "VERSION_SUFFIX_FOR_PYPI"
+ "--env" "VERSION_SUFFIX_FOR_SVN"
+ "--env" "CI"
Review comment:
How about using [--env-file](https://docs.docker.com/engine/reference/commandline/run/#set-environment-variables--e---env---env-file) ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #10158: Documentation artifact are also uploaded as GitHub Actions Artifacts
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #10158:
URL: https://github.com/apache/airflow/pull/10158#discussion_r465592569
##########
File path: scripts/ci/libraries/_initialization.sh
##########
@@ -131,7 +131,7 @@ function initialize_common_environment {
print_info
print_info "Mount whole airflow source directory for static checks"
print_info
- EXTRA_DOCKER_FLAGS=( \
+ EXTRA_DOCKER_FLAGS=(
"-v" "${AIRFLOW_SOURCES}:/opt/airflow" \
Review comment:
```suggestion
"-v" "${AIRFLOW_SOURCES}:/opt/airflow"
```
Also redundant? (no backslash in L:144)
----------------------------------------------------------------
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 #10158: Documentation artifact are also uploaded as GitHub Actions Artifacts
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #10158:
URL: https://github.com/apache/airflow/pull/10158#issuecomment-668636077
Ready to merge :)
----------------------------------------------------------------
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 #10158: Documentation artifact are also uploaded as GitHub Actions Artifacts
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #10158:
URL: https://github.com/apache/airflow/pull/10158#discussion_r465114325
##########
File path: scripts/ci/libraries/_initialization.sh
##########
@@ -148,6 +151,23 @@ function initialize_common_environment {
)
fi
+ # Common variables passed via docker commands
+ EXTRA_DOCKER_FLAGS+=(
+ "--env" "INSTALL_AIRFLOW_VERSION"
+ "--env" "PYTHONDONTWRITEBYTECODE"
+ "--env" "VERBOSE"
+ "--env" "VERBOSE_COMMANDS"
+ "--env" "HOST_USER_ID"
+ "--env" "HOST_GROUP_ID"
+ "--env" "HOST_OS"
+ "--env" "HOST_HOME"
+ "--env" "HOST_AIRFLOW_SOURCES"
+ "--env" "PYTHON_MAJOR_MINOR_VERSION"
+ "--env" "VERSION_SUFFIX_FOR_PYPI"
+ "--env" "VERSION_SUFFIX_FOR_SVN"
+ "--env" "CI"
Review comment:
I was under the impression that env-file requires var=value but I looked it up and we could indeed change it this way, Let me see if I can make it nicely :)
----------------------------------------------------------------
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 #10158: Documentation artifact are also uploaded as GitHub Actions Artifacts
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #10158:
URL: https://github.com/apache/airflow/pull/10158#issuecomment-669077621
> LGTM! But WOW... I didn't know that each build generates 1GB+ of artifacts
Those are recent changes - they were generated but never published as artifacts. And by looking at those - most of those are Html coverage reports. - 22 x 45 MB ~ 1GB :). Thsoe are hardly useful because they are partial coverage reports (only coverage.io when combining them might give us the final, good report. I will remove those in 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:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #10158: Documentation artifact are also uploaded as GitHub Actions Artifacts
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #10158:
URL: https://github.com/apache/airflow/pull/10158#issuecomment-669077621
> LGTM! But WOW... I didn't know that each build generates 1GB+ of artifacts
Those are recent changes - they were generated but never published as artifacts. And by looking at those - most of those are Html coverage reports. - 22 x 45 MB ~ 1GB :). Those are hardly useful because they are partial coverage reports (only coverage.io when combining them might give us the final, good report. I will remove those in 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:
users@infra.apache.org
[GitHub] [airflow] feluelle commented on a change in pull request #10158: Documentation artifact are also uploaded as GitHub Actions Artifacts
Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #10158:
URL: https://github.com/apache/airflow/pull/10158#discussion_r465572623
##########
File path: scripts/ci/libraries/_initialization.sh
##########
@@ -122,32 +118,44 @@ function initialize_common_environment {
HOST_OS="$(uname -s)"
export HOST_OS
+ # Home directory of the host user
+ HOST_HOME="${HOME}"
+ export HOST_HOME
+
+ # Sources of Airflow on the host.
+ HOST_AIRFLOW_SOURCES="${AIRFLOW_SOURCES}"
+ export HOST_AIRFLOW_SOURCES
# Add the right volume mount for sources, depending which mount strategy is used
if [[ ${MOUNT_SOURCE_DIR_FOR_STATIC_CHECKS} == "true" ]]; then
print_info
print_info "Mount whole airflow source directory for static checks"
print_info
EXTRA_DOCKER_FLAGS=( \
- "-v" "${AIRFLOW_SOURCES}:/opt/airflow" \
- "--env" "PYTHONDONTWRITEBYTECODE" \
+ "-v" "${AIRFLOW_SOURCES}:/opt/airflow" \
)
elif [[ ${MOUNT_HOST_AIRFLOW_VOLUME} == "true" ]]; then
print_info
print_info "Mounting necessary host volumes to Docker"
print_info
read -r -a EXTRA_DOCKER_FLAGS <<< "$(convert_local_mounts_to_docker_params)"
- EXTRA_DOCKER_FLAGS+=("-v" "${AIRFLOW_SOURCES}/files:/files" )
+ EXTRA_DOCKER_FLAGS+=( \
Review comment:
Does the backslash need to be there? Because in line 153 you didn't add it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #10158: Documentation artifact are also uploaded as GitHub Actions Artifacts
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #10158:
URL: https://github.com/apache/airflow/pull/10158
----------------------------------------------------------------
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 #10158: Documentation artifact are also uploaded as GitHub Actions Artifacts
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #10158:
URL: https://github.com/apache/airflow/pull/10158#discussion_r465147786
##########
File path: scripts/ci/libraries/_initialization.sh
##########
@@ -148,6 +151,23 @@ function initialize_common_environment {
)
fi
+ # Common variables passed via docker commands
+ EXTRA_DOCKER_FLAGS+=(
+ "--env" "INSTALL_AIRFLOW_VERSION"
+ "--env" "PYTHONDONTWRITEBYTECODE"
+ "--env" "VERBOSE"
+ "--env" "VERBOSE_COMMANDS"
+ "--env" "HOST_USER_ID"
+ "--env" "HOST_GROUP_ID"
+ "--env" "HOST_OS"
+ "--env" "HOST_HOME"
+ "--env" "HOST_AIRFLOW_SOURCES"
+ "--env" "PYTHON_MAJOR_MINOR_VERSION"
+ "--env" "VERSION_SUFFIX_FOR_PYPI"
+ "--env" "VERSION_SUFFIX_FOR_SVN"
+ "--env" "CI"
Review comment:
I did it and made it even nicer :).
----------------------------------------------------------------
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 #10158: Documentation artifact are also uploaded as GitHub Actions Artifacts
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #10158:
URL: https://github.com/apache/airflow/pull/10158#discussion_r465587913
##########
File path: scripts/ci/libraries/_initialization.sh
##########
@@ -122,32 +118,44 @@ function initialize_common_environment {
HOST_OS="$(uname -s)"
export HOST_OS
+ # Home directory of the host user
+ HOST_HOME="${HOME}"
+ export HOST_HOME
+
+ # Sources of Airflow on the host.
+ HOST_AIRFLOW_SOURCES="${AIRFLOW_SOURCES}"
+ export HOST_AIRFLOW_SOURCES
# Add the right volume mount for sources, depending which mount strategy is used
if [[ ${MOUNT_SOURCE_DIR_FOR_STATIC_CHECKS} == "true" ]]; then
print_info
print_info "Mount whole airflow source directory for static checks"
print_info
EXTRA_DOCKER_FLAGS=( \
- "-v" "${AIRFLOW_SOURCES}:/opt/airflow" \
- "--env" "PYTHONDONTWRITEBYTECODE" \
+ "-v" "${AIRFLOW_SOURCES}:/opt/airflow" \
)
elif [[ ${MOUNT_HOST_AIRFLOW_VOLUME} == "true" ]]; then
print_info
print_info "Mounting necessary host volumes to Docker"
print_info
read -r -a EXTRA_DOCKER_FLAGS <<< "$(convert_local_mounts_to_docker_params)"
- EXTRA_DOCKER_FLAGS+=("-v" "${AIRFLOW_SOURCES}/files:/files" )
+ EXTRA_DOCKER_FLAGS+=( \
Review comment:
Indeed. Fixed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #10158: Documentation artifact are also uploaded as GitHub Actions Artifacts
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #10158:
URL: https://github.com/apache/airflow/pull/10158#issuecomment-668635909
Hey - I updated doc building step to upload generated documentation as an artifact - this way you can very easily open the .zip file and see documentation generated by all PRs.
----------------------------------------------------------------
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 #10158: Documentation artifact are also uploaded as GitHub Actions Artifacts
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #10158:
URL: https://github.com/apache/airflow/pull/10158#issuecomment-669019224
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 #10158: Documentation artifact are also uploaded as GitHub Actions Artifacts
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #10158:
URL: https://github.com/apache/airflow/pull/10158#issuecomment-669080083
Removed here: https://github.com/apache/airflow/pull/10168
----------------------------------------------------------------
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