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/04/27 12:55:38 UTC
[GitHub] [airflow] ryw opened a new pull request #8586: Bump FAB in order to bump jQuery (resolves SNYK-JS-JQUERY-565129)
ryw opened a new pull request #8586:
URL: https://github.com/apache/airflow/pull/8586
To clear a [Cross-site Scripting (XSS) CVE](https://app.snyk.io/vuln/SNYK-JS-JQUERY-565129) in Airflow, looks like Airflow gets its jQuery from FAB.
FAB 2.3.3 bumps jQuery to resolve:
https://github.com/dpgaspar/Flask-AppBuilder/issues/1350
---
Make sure to mark the boxes below before creating PR: [x]
- [X] Description above provides context of the change
- [X] Unit tests coverage for changes (not needed for documentation changes)
- [X] Target Github ISSUE in description if exists
- [X] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [X] Relevant documentation is updated including usage instructions.
- [X] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
---
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).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
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] ryw commented on pull request #8586: Bump FAB in order to bump jQuery (resolves SNYK-JS-JQUERY-565129)
Posted by GitBox <gi...@apache.org>.
ryw commented on pull request #8586:
URL: https://github.com/apache/airflow/pull/8586#issuecomment-619969955
hmm maybe we should remove the jquery references in package.json too - will check
----------------------------------------------------------------
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] kaxil commented on a change in pull request #8586: Bump FAB in order to bump jQuery (resolves SNYK-JS-JQUERY-565129)
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #8586:
URL: https://github.com/apache/airflow/pull/8586#discussion_r416854768
##########
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:
I think these were a result of a rebase & force-push. The original commit did bump FAB version in setup.py. Hence needed this but I am sure this are not needed anymore
----------------------------------------------------------------
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] kaxil edited a comment on pull request #8586: Bump FAB in order to bump jQuery (resolves SNYK-JS-JQUERY-565129)
Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on pull request #8586:
URL: https://github.com/apache/airflow/pull/8586#issuecomment-620799992
I think we need to solve the underlying issue with FAB 2.3.3 (https://github.com/apache/airflow/issues/8613 & https://github.com/apache/airflow/issues/8599) before we can merge this as we just pinned FAB to 2.3.2 to fix those errors in https://github.com/apache/airflow/pull/8602
----------------------------------------------------------------
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] kaxil commented on pull request #8586: Bump FAB in order to bump jQuery (resolves SNYK-JS-JQUERY-565129)
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #8586:
URL: https://github.com/apache/airflow/pull/8586#issuecomment-620799992
I think we need to solve the underlying issue with FAB 2.3.3 (https://github.com/apache/airflow/issues/8613 & https://github.com/apache/airflow/issues/8599) before we can merge this
----------------------------------------------------------------
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] kaxil commented on pull request #8586: Bump FAB in order to bump jQuery (resolves SNYK-JS-JQUERY-565129)
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #8586:
URL: https://github.com/apache/airflow/pull/8586#issuecomment-620131658
@ryw Can you rebase on master again please
----------------------------------------------------------------
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 #8586: Bump FAB in order to bump jQuery (resolves SNYK-JS-JQUERY-565129)
Posted by 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
[GitHub] [airflow] ryw edited a comment on pull request #8586: Bump FAB in order to bump jQuery (resolves SNYK-JS-JQUERY-565129)
Posted by GitBox <gi...@apache.org>.
ryw edited a comment on pull request #8586:
URL: https://github.com/apache/airflow/pull/8586#issuecomment-620001209
ran the following commands and pushed
```
./breeze generate-requirements --python 3.6
./breeze generate-requirements --python 3.7
```
----------------------------------------------------------------
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] stale[bot] closed pull request #8586: Bump FAB in order to bump jQuery (resolves SNYK-JS-JQUERY-565129)
Posted by GitBox <gi...@apache.org>.
stale[bot] closed pull request #8586:
URL: https://github.com/apache/airflow/pull/8586
----------------------------------------------------------------
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] ryw commented on pull request #8586: Bump FAB in order to bump jQuery (resolves SNYK-JS-JQUERY-565129)
Posted by GitBox <gi...@apache.org>.
ryw commented on pull request #8586:
URL: https://github.com/apache/airflow/pull/8586#issuecomment-620714806
@potiuk when you get a chance, any ideas on what i need to do to clear build errors?
----------------------------------------------------------------
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 pull request #8586: Bump FAB in order to bump jQuery (resolves SNYK-JS-JQUERY-565129)
Posted by GitBox <gi...@apache.org>.
feluelle commented on pull request #8586:
URL: https://github.com/apache/airflow/pull/8586#issuecomment-620518920
jQuery 3.5.0 raises an issue in Airflow https://github.com/apache/airflow/issues/8599
----------------------------------------------------------------
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] kaxil edited a comment on pull request #8586: Bump FAB in order to bump jQuery (resolves SNYK-JS-JQUERY-565129)
Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on pull request #8586:
URL: https://github.com/apache/airflow/pull/8586#issuecomment-620131658
@ryw Can you rebase on master again please to fix the failing tests
----------------------------------------------------------------
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] ashb commented on a change in pull request #8586: Bump FAB in order to bump jQuery (resolves SNYK-JS-JQUERY-565129)
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #8586:
URL: https://github.com/apache/airflow/pull/8586#discussion_r416851707
##########
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:
I really think think we should update our process/automation to not touch these files when setup.py hasn't been touched either. (Ideally not unless the versions _in_ setup.py are updated.)
----------------------------------------------------------------
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] stale[bot] commented on pull request #8586: Bump FAB in order to bump jQuery (resolves SNYK-JS-JQUERY-565129)
Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #8586:
URL: https://github.com/apache/airflow/pull/8586#issuecomment-643476992
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
----------------------------------------------------------------
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] ryw commented on pull request #8586: Bump FAB in order to bump jQuery (resolves SNYK-JS-JQUERY-565129)
Posted by GitBox <gi...@apache.org>.
ryw commented on pull request #8586:
URL: https://github.com/apache/airflow/pull/8586#issuecomment-620001209
ran the follow commands and pushed
```
./breeze generate-requirements --python 3.6
./breeze generate-requirements --python 3.7
```
----------------------------------------------------------------
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 #8586: Bump FAB in order to bump jQuery (resolves SNYK-JS-JQUERY-565129)
Posted by 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 and the job will not try upgrade requirements eagerly if the hash has not changed. 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
[GitHub] [airflow] ashb commented on a change in pull request #8586: Bump FAB in order to bump jQuery (resolves SNYK-JS-JQUERY-565129)
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #8586:
URL: https://github.com/apache/airflow/pull/8586#discussion_r416867764
##########
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:
Makes sense, I thought that was what we did, I just hadn't seen the history of the PR so was a bit confused.
Thanks Jarek
----------------------------------------------------------------
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