-
Notifications
You must be signed in to change notification settings - Fork 16.4k
[AIRFLOW-2834] fix build script for k8s docker #3675
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
the travis build failed at check license, seams not a problem. |
ashb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests are failing as a result of the change though - it's complaining about all the new files that node has created.
You will need to add suitable excludion rules into the .rat-excludes file.
6439d8d to
83661f4
Compare
|
wired, the test complains the files in airflow/www_rbac/node_modules/ I have tried with different approach to make the exclude rule work, but failed. can you provide a simple instruction to how to ignore files under airflow/www_rbac/node_modules/ ? |
Codecov Report
@@ Coverage Diff @@
## master #3675 +/- ##
==========================================
+ Coverage 77.63% 77.64% +<.01%
==========================================
Files 204 204
Lines 15800 15800
==========================================
+ Hits 12267 12268 +1
+ Misses 3533 3532 -1
Continue to review full report at Codecov.
|
|
@verdan PTAL |
| airflow/www_rbac/node_modules | ||
| .*node_modules.* | ||
| .*json | ||
| flake8_diff.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think these changes should be in this PR as they are unrelated - (also flake8-diff.sh doesn't exist anymore)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I just saw my own previous comment. Sorry.
Still, flake8_diff.sh doesn't exist anymore
|
Are these changes needed for running tests, or are you re-using the images built there for running Airflow in kube? |
|
The only reason I hesitate as this will slow down the Travis builds a bit, and they are already slow enough, and this is not needed for running the tests. Would a comment, or a flag you pass |
|
Oh, and additionally/instead: If we do leave this in we should work out to add .node_modules to the Travis cache. |
|
@ashb sorry, I'm not very familiar with travis, this change is to make sure our kube change and kube unit test can pass through, and it's four month ago after I made this change... what do you mean by add some flag to cache? and how to add cache into travis? |
What unit tests depend on the front-end assets? |
|
I ran across this same issue and created a duplicate PR: #4248 While I understand these scripts are for CI, I also found using the Dockerfile and the kube deploy CI script convenient for testing & iterating on changes made to the kubernetes executor, etc. |
|
Couldn't you get even quicker dev cycles by volume-mounting the airflow code in? rather than having to rebuild? |
|
That'd likely work well. I could use the |
|
@ashb is that means we should not use this Dockerfile to build docker image of airflow, instead we should create one by ourself and then use it in k8s cluster? then I think we need to close this issue and then add some readme in the Dockerfile or some where |
|
Fixed by #3770 |
Make sure you have checked all steps below.
Jira
Description
the kubernetes docker build airflow without rbac support, but the configmap need rbac. so need to change the build script to build js and css files.
currently when open airflow web ui deployed in kubernetes, the webpage is blank and will be some file missing.
Tests
Commits
Documentation
Code Quality
git diff upstream/master -u -- "*.py" | flake8 --diff