Skip to content

Add Flask 2 support #186

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

Merged
merged 4 commits into from
Oct 25, 2022
Merged

Add Flask 2 support #186

merged 4 commits into from
Oct 25, 2022

Conversation

apotterri
Copy link
Contributor

Fixes getappmap/board/#148.

@apotterri apotterri requested review from symwell and dividedmind and removed request for symwell October 25, 2022 12:47
f"{{{p}}}" if c else p
for c, _, p in parse_rule(request.url_rule.rule)
]
# Transform request.url to the expected normalized-path form. For example,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dividedmind LMK if this doesn't satisfy your concerns about this change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, looks ok if a bit convoluted :) I'll propose a simpler and perhaps easier to read implementation

@apotterri
Copy link
Contributor Author

@symwell It's possible that c474ff9 fixes the issues with record-by-default in chypi.org.

f"{{{p}}}" if c else p
for c, _, p in parse_rule(request.url_rule.rule)
]
# Transform request.url to the expected normalized-path form. For example,
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, looks ok if a bit convoluted :) I'll propose a simpler and perhaps easier to read implementation

Instead of relying on an internal Flask API to parse a Rule, parse the
result of calling repr on it. The format of the string returned by repr
is the same for Flask 1 and Flask 2, and has all the information we
need.
Remove Flask 1 dependency from pyproject.toml. Instead, add a flask1
factor to tox.ini, and use it for testing Flask 1.
Fix up Flask support so it's compatible with v2. Also fixes initial
configuration, so a Flask app will be recorded by default.
Import appmap.django at startup, to make sure our middleware gets
injected.
@apotterri apotterri merged commit 329fee4 into master Oct 25, 2022
@apotterri apotterri deleted the flask2_20221017 branch October 25, 2022 21:21
@apotterri apotterri mentioned this pull request Nov 1, 2022
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