Skip to content

Bug 1562731 - upgrade to Python-3 - #246

Merged
djmitche merged 6 commits into
mozilla-services:masterfrom
djmitche:bug1562731
Nov 14, 2019
Merged

Bug 1562731 - upgrade to Python-3#246
djmitche merged 6 commits into
mozilla-services:masterfrom
djmitche:bug1562731

Conversation

@djmitche

Copy link
Copy Markdown
Contributor

The only laggard dependency was flask_secure_headers, which appears unmaintained. It's just setting headers, though, so not too complicated. I vendored tarek's version from twaldear/flask-secure-headers#9. He had finished the meat of it (really just removing some useless try/except's and fixing whitespace) and was working on the tests.

The tests pass locally. I bet they'll pass in CI. But I have less confidence this will work right-off in a deployment. We should probably figure out a plan to deploy "safely":

  • for the webapp, rollback is pretty easy and we can just QA "manually"
  • for the backend queue-monitoring bit, we can deploy it in a read-only fashion and see what it logs, thereby avoiding the risk of some subtle bug causing it to delete everyone's queues or something crazy.

Anyway, I'd like to get @escapewindow to have a look at this first, then we can work on landing it.

@escapewindow escapewindow left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stamping. If you need deeper flask review, that may not be me.

(I do see diffs if I clone Tarek's master into flask_secure_headers. The 3 patches after the vendor seem sane.)

@djmitche

Copy link
Copy Markdown
Contributor Author

Nothing flask-y changed here, so I think it's OK. I'll check the travis failures.

@ccooper

ccooper commented Nov 13, 2019

Copy link
Copy Markdown
Contributor

I checked the travis failures for you. :)

@djmitche

Copy link
Copy Markdown
Contributor Author

This has been rolled back in production. #251 appears unrelated (it reproduces with the patch rolled back) but #252 is definitely related.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants