Do not override in_container scripts when building the image #10442
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
boring-cyborg
bot
added
area:dev-tools
area:docs
area:Scheduler
Member
Author
mik-laj
approved these changes
Aug 21, 2020
kaxil
approved these changes
Aug 21, 2020
Member
kaxil
left a comment
kaxil
left a comment
There was a problem hiding this comment.
Just trusting you on this one :)
Member
Author
|
Yeah. This one has no chance to pass now as PR because it needs to be merged to get working but I am running the build in my own forks' master. The trouble with "workflow_run" that if we want to change anything in the build process, we need to push it to master somewhere to test it :( |
Member
Author
|
Main build here: https://github.com/potiuk/airflow/actions/runs/218264558 Once they pass some basic steps I will merge it. |
potiuk
force-pushed
the
do-not-override-in-container-scripts
branch
from
570fe48 to
84e747c
Compare
Member
Author
potiuk
force-pushed
the
do-not-override-in-container-scripts
branch
5 times, most recently
from
7746c77 to
48e2f2f
Compare
After apache#10368, we've changed the way we build the images on CI. We are overriding the ci scripts that we use to build the image with the scripts taken from master to not give roque PR authors the possibiility to run something with the write credentials. We should not override the in_container scripts, however because they become part of the image, so we should use those that came with the PR. That's why we have to move the "in_container" scripts out of the "ci" folder and only override the "ci" folder with the one from master. We've made sure that those scripts in ci are self-contained and they do not need reach outside of that folder. Also the static checks are done with local files mounted on CI because we want to check all the files - not only those that are embedded in the container.
potiuk
force-pushed
the
do-not-override-in-container-scripts
branch
from
48e2f2f to
179d6c2
Compare
Member
Author
|
Looks good: https://github.com/potiuk/airflow/runs/1012859548?check_suite_focus=true pushing it. I had to do another thing - disable the 6000 "skipped" tests in integration tests - they were not needed and with local mounts they caused "blocking I/O" on CI. But it works now :) |
potiuk
added a commit
to potiuk/airflow
that referenced
this pull request
Sep 14, 2020
…10442) After apache#10368, we've changed the way we build the images on CI. We are overriding the ci scripts that we use to build the image with the scripts taken from master to not give roque PR authors the possibiility to run something with the write credentials. We should not override the in_container scripts, however because they become part of the image, so we should use those that came with the PR. That's why we have to move the "in_container" scripts out of the "ci" folder and only override the "ci" folder with the one from master. We've made sure that those scripts in ci are self-contained and they do not need reach outside of that folder. Also the static checks are done with local files mounted on CI because we want to check all the files - not only those that are embedded in the container. (cherry picked from commit 1cf1af6)
potiuk
added a commit
that referenced
this pull request
Sep 15, 2020
After #10368, we've changed the way we build the images on CI. We are overriding the ci scripts that we use to build the image with the scripts taken from master to not give roque PR authors the possibiility to run something with the write credentials. We should not override the in_container scripts, however because they become part of the image, so we should use those that came with the PR. That's why we have to move the "in_container" scripts out of the "ci" folder and only override the "ci" folder with the one from master. We've made sure that those scripts in ci are self-contained and they do not need reach outside of that folder. Also the static checks are done with local files mounted on CI because we want to check all the files - not only those that are embedded in the container. (cherry picked from commit 1cf1af6)
RaviTezu
pushed a commit
to RaviTezu/airflow
that referenced
this pull request
Oct 25, 2020
…10442) After apache#10368, we've changed the way we build the images on CI. We are overriding the ci scripts that we use to build the image with the scripts taken from master to not give roque PR authors the possibiility to run something with the write credentials. We should not override the in_container scripts, however because they become part of the image, so we should use those that came with the PR. That's why we have to move the "in_container" scripts out of the "ci" folder and only override the "ci" folder with the one from master. We've made sure that those scripts in ci are self-contained and they do not need reach outside of that folder. Also the static checks are done with local files mounted on CI because we want to check all the files - not only those that are embedded in the container. (cherry picked from commit 1cf1af6)
kaxil
pushed a commit
that referenced
this pull request
Nov 12, 2020
After #10368, we've changed the way we build the images on CI. We are overriding the ci scripts that we use to build the image with the scripts taken from master to not give roque PR authors the possibiility to run something with the write credentials. We should not override the in_container scripts, however because they become part of the image, so we should use those that came with the PR. That's why we have to move the "in_container" scripts out of the "ci" folder and only override the "ci" folder with the one from master. We've made sure that those scripts in ci are self-contained and they do not need reach outside of that folder. Also the static checks are done with local files mounted on CI because we want to check all the files - not only those that are embedded in the container. (cherry picked from commit 1cf1af6)
potiuk
added
the
type:misc/internal
potiuk
added a commit
that referenced
this pull request
Nov 16, 2020
After #10368, we've changed the way we build the images on CI. We are overriding the ci scripts that we use to build the image with the scripts taken from master to not give roque PR authors the possibiility to run something with the write credentials. We should not override the in_container scripts, however because they become part of the image, so we should use those that came with the PR. That's why we have to move the "in_container" scripts out of the "ci" folder and only override the "ci" folder with the one from master. We've made sure that those scripts in ci are self-contained and they do not need reach outside of that folder. Also the static checks are done with local files mounted on CI because we want to check all the files - not only those that are embedded in the container. (cherry picked from commit 1cf1af6)
kaxil
added
provider:cncf-kubernetes
cfei18
pushed a commit
to cfei18/incubator-airflow
that referenced
this pull request
Mar 5, 2021
…10442) After apache#10368, we've changed the way we build the images on CI. We are overriding the ci scripts that we use to build the image with the scripts taken from master to not give roque PR authors the possibiility to run something with the write credentials. We should not override the in_container scripts, however because they become part of the image, so we should use those that came with the PR. That's why we have to move the "in_container" scripts out of the "ci" folder and only override the "ci" folder with the one from master. We've made sure that those scripts in ci are self-contained and they do not need reach outside of that folder. Also the static checks are done with local files mounted on CI because we want to check all the files - not only those that are embedded in the container. (cherry picked from commit 1cf1af6)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After #10368, we've changed the way we build the images
on CI. We are overriding the ci scripts that we use
to build the image with the scripts taken from master
to not give roque PR authors the possibiility to run
something with the write credentials.
We should not override the in_container scripts, however
because they become part of the image, so we should use
those that came with the PR. That's why we have to move
the "in_container" scripts out of the "ci" folder and
only override the "ci" folder with the one from
master. We've made sure that those scripts in ci
are self-contained and they do not need reach outside of
that folder.
Also the static checks are done with local files mounted
on CI because we want to check all the files - not only
those that are embedded in the container.
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.