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/06/12 13:04:14 UTC
[GitHub] [airflow] Khrol opened a new pull request #9250: Fix tree view if config contains "
Khrol opened a new pull request #9250:
URL: https://github.com/apache/airflow/pull/9250
If you run DAG with `{"\"": ""}` configuration tree view will be broken:
```
tree:1 Uncaught SyntaxError: Unexpected string in JSON at position 806
at JSON.parse (<anonymous>)
at tree?dag_id=hightlight_test&num_runs=25:1190
```
JSON.parse is given incorrectly escaped json string.
---
Make sure to mark the boxes below before creating PR: [x]
- [x] Description above provides context of the change
- [ ] Unit tests coverage for changes (not needed for documentation changes)
- [ ] Target Github ISSUE in description if exists
- [ ] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [ ] Relevant documentation is updated including usage instructions.
- [ ] 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] mik-laj commented on pull request #9250: Fix tree view if config contains "
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #9250:
URL: https://github.com/apache/airflow/pull/9250#issuecomment-644106773
It was green, but it was this build that ended three days ago. I am already working on it.
----------------------------------------------------------------
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] Khrol commented on pull request #9250: Fix tree view if config contains "
Posted by GitBox <gi...@apache.org>.
Khrol commented on pull request #9250:
URL: https://github.com/apache/airflow/pull/9250#issuecomment-644106125
@mik-laj tests were green on PR... Do you want me to fix tests or you can do it?
----------------------------------------------------------------
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 #9250: Fix tree view if config contains "
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #9250:
URL: https://github.com/apache/airflow/pull/9250#issuecomment-644121105
The problem with the CI in this build was that it only tested the static tests. As no ".py" file was changed, CI skipped rest of the unit test. However on Master we run all the tests, hence Master failed with below error:
```
tests/www/test_views.py .............................F
_________ TestAirflowBaseViews.test_escape_in_tree_view_0_hello_world __________
a = (<tests.www.test_views.TestAirflowBaseViews testMethod=test_escape_in_tree_view_0_hello_world>,)
@wraps(func)
def standalone_func(*a):
> return func(*(a + p.args), **p.kwargs)
/usr/local/lib/python3.6/site-packages/parameterized/parameterized.py:530:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/www/test_views.py:646: in test_escape_in_tree_view
self.check_content_in_response(f'"conf":{{"abc":"{seralized_test_str}"}}', resp)
tests/www/test_views.py:177: in check_content_in_response
self.assertIn(text, resp_html)
```
```
tests/www/test_views.py F
_________ TestAirflowBaseViews.test_escape_in_tree_view_1_hello_world __________
a = (<tests.www.test_views.TestAirflowBaseViews testMethod=test_escape_in_tree_view_1_hello_world>,)
@wraps(func)
def standalone_func(*a):
> return func(*(a + p.args), **p.kwargs)
/usr/local/lib/python3.6/site-packages/parameterized/parameterized.py:530:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/www/test_views.py:646: in test_escape_in_tree_view
self.check_content_in_response(f'"conf":{{"abc":"{seralized_test_str}"}}', resp)
tests/www/test_views.py:177: in check_content_in_response
self.assertIn(text, resp_html)
E AssertionError: '"conf":{"abc":"hello\\\\u0027world"}' not found in '\n\n\n\n\n\n\n\n\n\n
```
----------------------------------------------------------------
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] mik-laj commented on pull request #9250: Fix tree view if config contains "
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #9250:
URL: https://github.com/apache/airflow/pull/9250#issuecomment-644125021
I finished change: Here is PR: https://github.com/apache/airflow/pull/9307
----------------------------------------------------------------
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] Khrol commented on pull request #9250: Fix tree view if config contains "
Posted by GitBox <gi...@apache.org>.
Khrol commented on pull request #9250:
URL: https://github.com/apache/airflow/pull/9250#issuecomment-643338385
@mik-laj I've checked the code by `JSON.parse` calls. Nothing similar.
----------------------------------------------------------------
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] mik-laj commented on pull request #9250: Fix tree view if config contains "
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #9250:
URL: https://github.com/apache/airflow/pull/9250#issuecomment-643282850
Have you checked whether the problem also occurs elsewhere? Similar problems like to be repeated.
----------------------------------------------------------------
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] mik-laj commented on pull request #9250: Fix tree view if config contains "
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #9250:
URL: https://github.com/apache/airflow/pull/9250#issuecomment-644099285
It seems that this change has broken the tests on the master branch.
Last valid build on master:
https://github.com/apache/airflow/runs/772104379
Next finished build on master:
https://github.com/apache/airflow/runs/772331104
----------------------------------------------------------------
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 #9250: Fix tree view if config contains "
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #9250:
URL: https://github.com/apache/airflow/pull/9250#issuecomment-643496516
Yeah, this used to be just `var data = {{ data | tojson }}` but for _large_ dags (i.e. AirB'n'B scale) this was a slow down on rendering the web page.
----------------------------------------------------------------
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 merged pull request #9250: Fix tree view if config contains "
Posted by GitBox <gi...@apache.org>.
ashb merged pull request #9250:
URL: https://github.com/apache/airflow/pull/9250
----------------------------------------------------------------
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