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

Fix the request blueprint method #4059

Closed
wants to merge 2 commits into from
Closed

Fix the request blueprint method #4059

wants to merge 2 commits into from

Conversation

pgjones
Copy link
Member

@pgjones pgjones commented May 15, 2021

Previously for a nested blueprint it would return the parent_name.name
rather than the name of the current blueprint. This is fixed and
another blueprints method is added in order to get the parent_name and
name. The latter is made use of in the app.

I've used a cached_property as this method is called a few times per
request (by the app methods).

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

CHANGES.rst Outdated Show resolved Hide resolved
Previously for a nested blueprint it would return the parent_name.name
rather than the name of the current blueprint. This is fixed and
another blueprints method is added in order to get the parent_name and
name. The latter is made use of in the app.

I've used a cached_property as this method is called a few times per
request (by the app methods).
It now takes account of the fact that blueprints can be nested.
@davidism
Copy link
Member

Based on #4069, I think we need some more thought about what a blueprint's name is. Right now request.blueprint is correct for what would be needed for #4069: a blueprint's name should be "unique" in that it's the full path it's registered on the app with. That also feels adjacent.

Additionally, this implementation of blueprints reverses the names, so that it's in the opposite order of the endpoint name. Perhaps the user-facing property needs to return in the original order, so request.blueprint == request.blueprints[-1], and an _iter_blueprints method is available for internal use.

@pgjones
Copy link
Member Author

pgjones commented May 17, 2021

At the moment though the blueprint is registered on the app with its name, which is what request.blueprint will return if this is merged i.e. I thought this should continue work app.blueprints[request.blueprint] for nested blueprints.

Yea, I'd wondered about the ordering. I thought that given this method (and request.blueprint) mostly exist for flask internal usage it was ok to default this way. Happy to switch though.

@pgjones pgjones closed this May 17, 2021
@pgjones pgjones deleted the bps branch May 17, 2021 21:11
@pgjones
Copy link
Member Author

pgjones commented May 17, 2021

An alternative will be proposed, based on recent issue reports.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants