sanic-org / sanic

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

Support overwriting a route of blueprint from copy #2554

Closed Tpinion closed 1 year ago

Tpinion commented 1 year ago

Feture Description I want to overwrite the implement of route partially when they belong to different version blueprint, and it raises sanic_routing.exceptions.RouteExists. Sample Code

from sanic import Blueprint
from sanic.response import json
from sanic import Sanic

app = Sanic('test')

bpv1 = Blueprint('bpv1', version=1)

@bpv1.route('/hello')
async def root(request):
    return json('hello v1')

app.blueprint(bpv1)

bpv2 = bpv1.copy('bpv2', version=2)

@bpv2.route('/hello')
async def root(request):
    return json('hello v2')

app.blueprint(bpv2)

Current Solution I get this solution from forum and i was encouraged to post new a feature request based on it~

bpv2 = bpv1.copy("bpv2", version=2)

bpv2._future_routes = {
    route for route in bpv2._future_routes if route.uri != "/hello"
}

@bpv2.route("/hello")
async def root2(request):
    return json("hello v2")

Web Link https://community.sanicframework.org/t/how-to-overwrite-a-route-when-using-blueprint-copy/1067

ahopkins commented 1 year ago

Thanks for adding this. As I mentioned in that thread, I think my preferred solution to this would be to set a flag on the Blueprint instance that would allow a route to overwrite a duplicate. This should be False all the time and probably only turned on through the copy API:

new_bp = old_bp.copy(allow_overwrite=True)

We would probably need to change Blueprint.route to handle this.

Another option might be to bring back remove_route. @Garito might roll his eyes at the suggestion. As a bit of background, there used to be app.remove_route. It was deprecated in 2019, and removed in 2020. Lots of discussion on GitHub and the Forums. Pinging also @sjsadowski and @vltr who were vocal in the conversation.

IIRC the reason it was removed was the conflict is was causing people with servers while running and syncing across processes. We potentially could bring it back with the following condition:

  1. It fails if server is running (much easier to determine that today than it was in 2019)
  2. It also has a force flag if someone needs to do it (let the dev add protection from shooting themselves in their own feet)

In favor of this idea: it is actually much easier now to also share context among workers. It is much more likely that someone could cover this issue.


I am up for other suggestions, but it strikes me that the underlying principle might be best served with the narrow allow_overwrite option. However, remove_route does not seem as outlandish to me now as it did to me a couple years ago. :thinking:

ChihweiLHBird commented 1 year ago

@ahopkins Why don't we allow overwriting routes globally even for a non-copied blueprint?

ahopkins commented 1 year ago

Correct. For that reason, remove_route doesn't sound like a terrible idea. Assuming--of course--it is happening before router finalization.

ChihweiLHBird commented 1 year ago

@ahopkins I am interested in working on this and explore the way to allowing overwriting. Is there anyone else working on it?

ahopkins commented 1 year ago

nope