[GitHub] [airflow] potiuk commented on a change in pull request #13313: Warns politely, do not force run a long operation

[GitBox <gi...@apache.org>]

potiuk commented on a change in pull request #13313:
URL: https://github.com/apache/airflow/pull/13313#discussion_r548962702



##########
File path: airflow/www/compile_assets.sh
##########
@@ -20,10 +20,15 @@ set -e
 
 cd "$( dirname "${BASH_SOURCE[0]}" )"
 
+MD5SUM_FILE="static/dist/sum.md5"
+readonly MD5SUM_FILE
+
 # first bump up package.json manually, commit and tag
 if [[ -d ./static/dist ]]; then
   rm -f ./static/dist/*
 fi
 
 yarn install --frozen-lockfile
 yarn run build
+
+find package.json yarn.lock static/css static/js -type f | sort | xargs md5sum > "${MD5SUM_FILE}"

Review comment:
       One more thing. In order this to work properly, also `airflow/www/compile_assets.sh` should be run as step in `Dockerfile.ci`
   
   https://github.com/apache/airflow/blob/6f246b0d54ccaf733b7c5951a8955adda6719acb/Dockerfile.ci#L348
   
   In the Dockefile.ci we this in two steps (this is an optimisation of image rebuilding)
   
   1) first we install node_modules (in case .lock/ or package.json changed): https://github.com/apache/airflow/blob/6f246b0d54ccaf733b7c5951a8955adda6719acb/Dockerfile.ci#L300
   
   2) then we update all latest setup.py dependencies in case setup.py changed https://github.com/apache/airflow/blob/6f246b0d54ccaf733b7c5951a8955adda6719acb/Dockerfile.ci#L322
   
   3) only then we build assets (only if any www files changed): https://github.com/apache/airflow/blob/6f246b0d54ccaf733b7c5951a8955adda6719acb/Dockerfile.ci#L348
   
   However there is a difference here: 
   
   1) we use `yarn prod` in the Dockerfile.ci
   2) we use `yarn build` in the compile_assets.sh
   
   There are two things to fix here:
   
   1) We should decide if we should use `build` or `prod' and use the same in both cases (AFAIK prod is optimized version of assets) (@ryanahamilton WDYT ?). I'd vote for `build` as it produces un-obfuscated code if that's the case..
   
   2) We should calculate and store the md5sum also in case of Dockerfile.ci build (because otherwise it will be always rebuilt).
   
   Do you want to do that @mik-laj ? Or should I do that myself?
   
   
   




----------------------------------------------------------------
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