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

Upper bound on Flask and Werkzeug versions #2538

Merged
merged 9 commits into from
May 24, 2023
Merged

Conversation

alexcjohnson
Copy link
Collaborator

@alexcjohnson alexcjohnson commented May 23, 2023

A less drastic alternative to #2516 - simply set an upper bound on Flask and Werkzeug at versions that we've tested thoroughly and we know work.

We're well aware of the arguments against upper version bounds - articulated most clearly perhaps in https://iscinumpy.dev/post/bound-version-constraints/ - let me just point out one line in the intro of that piece:

After reading this, hopefully you will always consider every cap you add, you will know the (few) places where pinning an upper limit is reasonable

I think we have ample evidence at this point that this is such a place. From our experience with the last several Flask minor releases, this particular upper bound expresses a true statement about Flask & Dash: We expect the Dash ecosystem to be incompatible with the next minor release of Flask. We know that this is true for Flask 2.3 already, and we are working to fix the incompatibilities in our related libraries, after which we will bump these upper limits to <2.4. Note that we need to limit both Flask and Werkzeug, because even though they're normally released synchronously with matching version numbers, Flask depends on Werkzeug>={matching version}

After we make this change, we will institute a process upon every new Flask minor release where we thoroughly test it across the Dash ecosystem, make any required changes, and then bump the upper version constraint. In general we will bump to <{next minor} rather than <={known good patch} as we're doing here, just because the later patches Flask@2.2.4 and Flask@2.2.5 that currently exist are primarily about forward compatibility with Werkzeug@2.3.x which we are not supporting in the upcoming Dash release.

@alexcjohnson
Copy link
Collaborator Author

html build is failing:

Unexpected number of elements extracted from https://developer.mozilla.org/en-US/docs/Web/HTML/Element - Found 122 but expected 125 Check the output and edit expectedElCount if this is intended.

Working on it, I'll address this in this PR as well.

@T4rk1n T4rk1n mentioned this pull request May 23, 2023
CHANGELOG.md Outdated
@@ -4,6 +4,10 @@ This project adheres to [Semantic Versioning](https://semver.org/).

## [UNRELEASED]

## Changed

- [#2538](https://github.com/plotly/dash/pull/2538) Add an upper bound to Flask and Werkzeug versions at `<2.2.3`, to reflect the fact that we expect the Dash ecosystem to be incompatible with the next minor release of Flask. This excludes the current latest Flask release 2.3.x, we will raise that to `<2.4` after we fix incompatibilities elsewhere in the Dash ecosystem.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpicking but there's a comma splice in the second sentence. I'd suggest different punctuation, but it's not clear to me how the two clauses relate to each other―if they don't really, maybe something like

Add an upper bound to Flask and Werkzeug versions at <2.2.3 because we expect the Dash ecosystem to be incompatible with the next minor release of Flask (this excludes the current latest Flask release 2.3.x). We will raise the upper bound to <2.4 after we fix incompatibilities elsewhere in the Dash ecosystem.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated in c25225b - thanks @red-patience!

@@ -76,7 +76,7 @@ export function getPath(paths, id) {
return false;
}
const values = props(keys, id);
const pathObj = find(propEq('values', values), keyPaths);
const pathObj = find(propEq(values, 'values'), keyPaths);
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah it was inverted, strange because it feel more natural with the prop name first.

Copy link
Contributor

@T4rk1n T4rk1n left a comment

Choose a reason for hiding this comment

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

💃

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