[GitHub] [airflow] potiuk commented on a change in pull request #8586: Bump FAB in order to bump jQuery (resolves SNYK-JS-JQUERY-565129)

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

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



##########
File path: requirements/requirements-python3.6.txt
##########
@@ -43,7 +43,7 @@ apispec==1.3.3
 appdirs==1.4.3
 argcomplete==1.11.1
 asn1crypto==1.3.0
-astroid==2.3.3
+astroid==2.4.0

Review comment:
       Yes it was. And it's currently done as you expect @ashb -> every time 'generate requirements" is run, the "setupN.N.txt" is generated with md5hash of setup.py. So only after setup.py is changed, you should run the "generate-requirements" to regenerate requirements and the setup*.txt file. 
   
   In fact even now if we regenerate the requirements always in the cron job, it is purely information in the logs - it does not have to be fixed until the next setup.py modification and the job never fails when requirement files are modified by it.  But it might be helpful if cron job will start failing because of dependency update - then we will see in the logs clearly which requirements were upgraded by the CRON job, so that we can pin-point the culprit easily and know how to fix it quickly.
   
   In this case, it was indeed fully justified as there were two parallel modifications of the setup.py in two different branches. And failing build correctly protected us from potentially conflicting/wrong requirements. So I think all worked as expected.




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