Skip to content
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.

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

AN-296 Upgrade Werkzeug to 3.0.6 #796

Merged
merged 22 commits into from
Jan 2, 2025
Merged

AN-296 Upgrade Werkzeug to 3.0.6 #796

merged 22 commits into from
Jan 2, 2025

Conversation

jgainerdewar
Copy link
Collaborator

@jgainerdewar jgainerdewar commented Dec 12, 2024

This upgrade ended up being, uh, nontrivial.

  • The Werkzeug 2 -> 3 upgrade also resulted in Flask and Connexion needing the same major version upgrade, and Connexion's update required code changes.
  • The Flask update from 2 -> 3 broke flask_testing, which seems to be abandonware, so updated tests to use plain unittest instead.
  • Needed to re-order endpoints in the swagger yaml to put narrower definitions at the beginning (ex. /jobs/operationDetails before /jobs/{id}), otherwise parameter parsing failed.
  • I removed the special handling for PyYAML because I see no evidence that we still need it.

@jgainerdewar jgainerdewar marked this pull request as draft December 13, 2024 14:39
@jgainerdewar jgainerdewar marked this pull request as ready for review December 17, 2024 22:01
Copy link
Collaborator

@aednichols aednichols left a comment

Choose a reason for hiding this comment

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

Nice work!

High time to replace Job Manager so we don't do this again in two years.

Copy link

@LizBaldo LizBaldo left a comment

Choose a reason for hiding this comment

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

Amazing, thanks for figuring this out 😱 I only have a couple of minor comments, lmk what you think.

api/jobs.yaml Show resolved Hide resolved
from ..__main__ import loadCapabilities
from ..models.capabilities_response import CapabilitiesResponse
from . import BaseTestCase
from . import create_app

Choose a reason for hiding this comment

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

Nit, and not necessarily something to tackle in this PR as it seemed to have n=been the prior behavior, but using relative paths to import modules are not very robust, and setting up each init.py file so that we can import like from test import create_app would be a lot nicer

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed... I'm not going to do that in this PR, I think there's enough churn already, but good to keep in mind next time we're in here.

@jgainerdewar jgainerdewar merged commit 11a04df into master Jan 2, 2025
5 checks passed
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