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 2021/03/10 15:05:10 UTC
[GitHub] [airflow] kaxil opened a new pull request #14698: Separate out tests to cater of changes in Python 3.8.8
kaxil opened a new pull request #14698:
URL: https://github.com/apache/airflow/pull/14698
https://github.com/python/cpython/pull/24297 change was included in
Python 3.8.8 to fix a vulnerability (bpo-42967)
Depending on which Base Python Image is run in our CI, two of the tests
can fail or succeed.
Our Previous two attempts:
- https://github.com/apache/airflow/commit/061cd236deb22567e4de36af11025f028d787989#
- https://github.com/apache/airflow/commit/49952e79b04da932242ebf3981883e591b467994
We might for a while get different base python version depending on the changes of a PR (whether or not it includes a change to dockerfiler).
a) when you have PR which do not have changes in the Dockerfile, they will use the older python version as base (for example Python 3.8.7)
b) when you have PR that touches the Dockerfile and have setup.py changes in master, it should pull Python 3.8.8 first.
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
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).
----------------------------------------------------------------
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 pull request #14698: Separate out tests to cater for changes in Python 3.8.8
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #14698:
URL: https://github.com/apache/airflow/pull/14698#issuecomment-796577175
Nice! Thanks @kaxil !
----------------------------------------------------------------
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 pull request #14698: Separate out tests to cater for changes in Python 3.8.8
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #14698:
URL: https://github.com/apache/airflow/pull/14698#issuecomment-795862490
> Depending on which Base Python Image is run in our CI, two of the tests
You mean this isn't predictable?
----------------------------------------------------------------
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 #14698: Separate out tests to cater of changes in Python 3.8.8
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #14698:
URL: https://github.com/apache/airflow/pull/14698#discussion_r591600386
##########
File path: tests/www/test_views.py
##########
@@ -2784,7 +2784,38 @@ def test_trigger_dag_form(self):
("%2Fgraph%3Fdag_id%3Dexample_bash_operator", "/graph?dag_id=example_bash_operator"),
]
)
- def test_trigger_dag_form_origin_url(self, test_origin, expected_origin):
+ @pytest.mark.skipif(
+ sys.version_info < (3, 8, 8),
Review comment:
I did not do:
```python
sys.version_info <= (3, 8, 7)
```
for the following case:
```python
❯ python
Python 3.7.4 (default, Aug 13 2019, 15:17:50)
[Clang 4.0.1 (tags/RELEASE_401/final)] :: Anaconda, Inc. on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.version_info == (3, 7, 4, )
False
>>> sys.version_info
sys.version_info(major=3, minor=7, micro=4, releaselevel='final', serial=0)
>>> sys.version_info == (3, 7, 4, 'final', 0)
True
```
----------------------------------------------------------------
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 #14698: Separate out tests to cater for changes in Python 3.8.8
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #14698:
URL: https://github.com/apache/airflow/pull/14698#issuecomment-796194898
> > Depending on which Base Python Image is run in our CI, two of the tests
>
> You mean this isn't predictable?
Yup, from https://github.com/apache/airflow/blob/6851677a89294698cbdf9fa559bf9d12983c88e0/scripts/ci/libraries/_push_pull_remove_images.sh#L107-L146
Jarek has explained and he is going to fix that in a follow-up PR
----------------------------------------------------------------
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] github-actions[bot] commented on pull request #14698: Separate out tests to cater for changes in Python 3.8.8
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14698:
URL: https://github.com/apache/airflow/pull/14698#issuecomment-796274175
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest master or amend the last commit of the PR, and push it with --force-with-lease.
----------------------------------------------------------------
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] XD-DENG commented on a change in pull request #14698: Separate out tests to cater for changes in Python 3.8.8
Posted by GitBox <gi...@apache.org>.
XD-DENG commented on a change in pull request #14698:
URL: https://github.com/apache/airflow/pull/14698#discussion_r591712047
##########
File path: tests/www/test_views.py
##########
@@ -3330,7 +3361,36 @@ class TestHelperFunctions(TestBase):
]
)
@mock.patch("airflow.www.views.url_for")
- def test_get_safe_url(self, test_url, expected_url, mock_url_for):
+ @pytest.mark.skipif(
+ sys.version_info < (3, 8, 8),
+ reason='Vulnerability was fixed in Python 3.8.8 which changed the query string separator: bpo-42967',
+ )
+ def test_get_safe_url_py_lte_387(self, test_url, expected_url, mock_url_for):
+ mock_url_for.return_value = "/home"
+ with self.app.test_request_context(base_url="http://localhost:8080"):
+ assert get_safe_url(test_url) == expected_url
+
+ @parameterized.expand(
+ [
+ ("", "/home"),
+ ("http://google.com", "/home"),
+ (
+ "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3Fdag_id%test_dag';alert(33)//",
+ "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3F"
+ "dag_id%25test_dag%27&alert%2833%29%2F%2F=",
Review comment:
This line is missed to be 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] kaxil merged pull request #14698: Separate out tests to cater for changes in Python 3.8.8
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #14698:
URL: https://github.com/apache/airflow/pull/14698
----------------------------------------------------------------
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