search

sanic-org / sanic

Accelerate your web app development | Build fast. Run fast.
https://sanic.dev
MIT License
18k stars 1.54k forks source link

Deprecate __blueprintname__ #2442

Closed ahopkins closed 1 year ago

ahopkins commented 2 years ago

This code no longer seems like it is necessary: https://github.com/sanic-org/sanic/blob/5d683c6ea4b615e80c51d80189436437b824cce6/sanic/blueprints.py#L309

Since you can get access to the name of the blueprint from the request object, the route object, and the blueprint object, I do not think there is any need for __blueprintname__ anymore.

See #2440

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is incorrect, please respond with an update. Thank you for your contributions.

r0x0d commented 1 year ago

Hello, @ahopkins. Since I'm experiencing the same error as mentioned in #2440, and, making the functions static works just fine for me too, I would prefer to keep my functions the way they are.

That said, I would like to open a PR to fix this issue, but I have never contributed to the Sanic codebase before, and I'm not 100% sure of what to do.

What are your suggestions to fix this? Removing the future.handler.__blueprintname__ = self.name completely, or do you have anything else in mind for this? If you have any directions for me, I would be happy to open a PR to fix that.

ahopkins commented 1 year ago

Awesome. I would love to have your first contribution, and happy to walk you thru it. There is some information already, but certainly reach out and let me know if you have questions.

As for the specifics here, I think we probably can just remove it. However I am reluctant for backwards compat to do anything before v23 at this point. One of the things I would like to introduce is to wrap all handlers with some sort of a parent object to make the API a little more consistent and we do not do hacky things like adding properties to functions.

r0x0d commented 1 year ago

Thanks, @ahopkins! I will see if I can provide a patch by today.

I tested locally with a project, and removing that line worked just fine for me. Not sure if it broke anything else, but I will see if I can do that today and give that a proper test.