Skip to content

update upper bound for MarkupSafe #19953

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
uranusjr merged 2 commits into from
Dec 6, 2021
Merged

update upper bound for MarkupSafe #19953

uranusjr merged 2 commits into from
Dec 6, 2021

Conversation

@tsingh2k15
Copy link
Contributor

@tsingh2k15 tsingh2k15 commented Dec 2, 2021

Per discussion and guidance from #19753, opening this PR for review. Based on if all the tests pass, this could be reviewed further. Resolves #19761.

@potiuk
Copy link
Member

potiuk commented Dec 6, 2021

Looks like

tests/www/views/test_views_rendered.py::test_user_defined_filter_and_macros_raise_error: AssertionError

Is the one that should be looked at - other errors are intermittent, but this one looks like related.

@potiuk
Copy link
Member

potiuk commented Dec 6, 2021

You can easily reproduce it in Breeze - (see the error log for instructions - you can also build your own image with breeze --upgrade-to-newer-dependencies in your PR and you should be able to reproduce it (and fix hopefully).

@uranusjr
Copy link
Member

uranusjr commented Dec 6, 2021

Weird, I just checked out this locally and can’t reproduce. Let me merge main and see if it’s just something wrong in the CI workers.

@uranusjr
Copy link
Member

uranusjr commented Dec 6, 2021

Oh never mind I got it.

@uranusjr
Copy link
Member

uranusjr commented Dec 6, 2021

I pushed a change to fix that test failure.

potiuk
potiuk reviewed Dec 6, 2021
View reviewed changes
tests/www/views/test_views_rendered.py
"debugging the rendering of template_fields.<br><br>"
) in resp_html

# MarkupSafe changed the exception detail from 'no filter named' to
Copy link
Member

@potiuk potiuk Dec 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:D

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Dec 6, 2021
@github-actions
Copy link

github-actions bot commented Dec 6, 2021

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@uranusjr uranusjr merged commit ba6b7c7 into apache:main Dec 6, 2021
@boring-cyborg
Copy link

boring-cyborg bot commented Dec 6, 2021

Awesome work, congrats on your first merged pull request!

@tsingh2k15
Copy link
Contributor Author

tsingh2k15 commented Dec 6, 2021

I am sorry, just looked at the messages, if this change is accepted, I would prefer to completely lift off the upper bound rather than restricting it to <=2.0. Is this feasible to lift off the upper bound in separate PR? Thoughts? Thank you for your support @potiuk @uranusjr

@potiuk
Copy link
Member

potiuk commented Dec 6, 2021

I am sorry, just looked at the messages, if this change is accepted, I would prefer to completely lift off the upper bound rather than restricting it to <=2.0. Is this feasible to lift off the upper bound in separate PR? Thoughts? Thank you for your support @potiuk @uranusjr

Sure - go gor it

@tsingh2k15
Copy link
Contributor Author

tsingh2k15 commented Dec 7, 2021

I am sorry, just looked at the messages, if this change is accepted, I would prefer to completely lift off the upper bound rather than restricting it to <=2.0. Is this feasible to lift off the upper bound in separate PR? Thoughts? Thank you for your support @potiuk @uranusjr

Sure - go gor it

Thanks, I will monitor the test runs for #20113. If there are issues, will close the PR.

@potiuk potiuk added this to the Airflow 2.2.3 milestone Dec 8, 2021
jedcunningham pushed a commit that referenced this pull request Dec 8, 2021
@tsingh2k15 @uranusjr @jedcunningham
update upper bound for MarkupSafe (#19953)
486dbae 
Co-authored-by: Tzu-ping Chung <tp@astronomer.io>
(cherry picked from commit ba6b7c7)
@jedcunningham jedcunningham added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Dec 8, 2021
@potiuk potiuk modified the milestones: Airflow 2.2.3, Airflow 2.2.4 Jan 22, 2022
potiuk pushed a commit that referenced this pull request Jan 22, 2022
@tsingh2k15 @uranusjr @potiuk
update upper bound for MarkupSafe (#19953)
7dfa24b 
Co-authored-by: Tzu-ping Chung <tp@astronomer.io>
(cherry picked from commit ba6b7c7)
jedcunningham pushed a commit that referenced this pull request Jan 27, 2022
@tsingh2k15 @uranusjr @jedcunningham
update upper bound for MarkupSafe (#19953)
5f1236f 
Co-authored-by: Tzu-ping Chung <tp@astronomer.io>
(cherry picked from commit ba6b7c7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@potiuk potiuk potiuk approved these changes

Assignees

No one assigned

Labels

changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) full tests needed We need to run full set of tests for this PR to merge

Projects

None yet

Milestone

Airflow 2.2.4

Development

Successfully merging this pull request may close these issues.

removing upper bound for markupsafe

4 participants

@tsingh2k15 @potiuk @uranusjr @jedcunningham