Removing routes: necessary functionality?

Moving forward Feature Requests
#1

Recently, a user raised a question regarding documenting the removal of routes in Sanic.

Following @sjsadowski request, I’m creating here a discussion regarding this item.

IMO, I don’t think we need such feature inside Sanic. The first question regarding this is why having to remove a route a feature anyway? You just don’t need to create it. Let me elaborate more regarding some different perspectives.

Complexity

As most of us know, the Sanic router itself is already rather complex, with a lot of different set of rules for dynamic routes, static routes, versioned routes, optional strict slashes and so on. Now, add to the mix the ability to group and nest blueprints. Yes, route creation rules became complex as well.

So, imagine the following scenario: deleting a route from a nested blueprint that was created using the version argument. Yummy.

State management

Ok, now the second problem in case you want to remove a route: how to provide a proper state management in your deploy, if you have the possibility to scale your application, vertically and horizontally? route.remove("/top-secret") will have to be orchestrated between all running workers, or else we might end up with an issue saying “Remove routing [doesn’t work | works partially] in production”, stating that “all tests works fine in the development environment”. Blame on us for providing such functionality without thinking about all possible caveats the user might find way further down the road.

A very similar question was made in another issue, just as a heads up.

Wrapping up

As long as I want to provide happiness to all Sanic users, I must say that some things are just too off hand to contemplate the full aspect of the word “solution”, thus questioning if this functionality should stay at Sanic.

Let me know your thoughts.

My best regards,
Richard.

1 Like
#2

I don’t think removing routes is necessary and would propose we deprecate the feature in 18.12 and remove for 19.X

1 Like
#3

Yeap, I think this might work as is the logical thing to do: deprecate and remove :+1:

2 Likes
#4

I’m going to submit a PR to deprecate this feature - provided I don’t find it in use elsewhere. There were no real objections in here. I think we slipped on deprecating in 18.12, which is fine, but we can deprecate in 19.03 and then remove for 19.12.

1 Like
#5

I think it a good practice to start depracation warnings in the minor releases and do the actual change in the LTS. Or, to at least give one intermediate release.

#6

I think I might agree with @sjsadowski on that. We can (and should) deprecate on 19.03, leave the deprecation warning for some time until the next LTS release, when it’ll be completely removed (and give us time to document any hack / workaround that some users might use given the limitations (of course).

#7

Good day.
I am using Sanic to build workflow application builder with headless CMS functionality.
One of routes are used by sanic_graphql.
The GraphQL schema is constantly changed. And i have to re-create schema and Sanic route on every change.

Route is created like so -

def init_graphql_view(): 
    this.graphql_view = GraphQLView.as_view(schema=ax_schema.schema,
                                              graphiql=False,
                                              executor=AsyncioExecutor(loop=this.loop))
    this.app.add_route(this.graphql_view, '/api/graphql')

And to reload schema i have to remove route and add it again with new schema -

app.remove_route(’/api/graphql’)
init_graphql_view()

I hope you would not deprecate remove_route functionality. Or at least provide a workaround.
Thanks.

1 Like
#8

@enf644 Thank you very much for the feedback and sharing your use case.

So I understand, you are constantly adding and removing the same route? I recently started playing with some changes to router, so it would be good to have this in mind.

How do you handle the issue above with multiple running instances? or is it a single instance? I think the question and problem is that this feature is a trap for scalability.

#9

Unfortunately, i did not try this with multiple instances. Not sure how i will handle this issues.
But i could not figure another way to modify route view class parameters.
Maybe you could point me to the right direction.

I made a simple view class -

class DummyView(HTTPMethodView):
    schema = None

    def __init__(self, **kwargs):
        for key, value in kwargs.items():
            if hasattr(self, key):
                setattr(self, key, value)

    def get(self, request, *args, **kwargs):
        return response.text(f'I am get method with {self.schema}')

And I add route like so -

dummy_view = DummyView.as_view(schema='Hello world')
app.add_route(dummy_view, '/api/help')

Is it possible to change a schema parameter of view class without removing and re-creating route?

#10

I have a thought… I’ll try it out tomorrow and let you know if it works.

#11

My idea was to create some sort of a signal or shared memory store for each of the processes. It could be RabbitMQ, mmap, or even something like what is proposed in PEP 554 if it ever lands in a future python release. I chose to use redis based pubsub because I am a fan of it, and I find it to be a great tool alongside Sanic.

I created a gist with the full code.

The basic idea is that each worker process starts up a receiver whose sole job is to get the signal that the schema needs updating:

raw = await app.pubsub.wait_message()

Now, I am not sure how or when you are deciding to update your schema. In my example, it happens with a POST request.

async def post(self, request):
    schema = request.json.get("schema")

Once we have that, we are just going to serialize it and pass a signal to let all processes know that we are updating the schema for this view: self.__class__.__name__.

In my example, with redis, it looks like this:

await request.app.redis.publish(
    CHANNEL, json.dumps({self.__class__.__name__: schema})
)

Going back to that worker’s receiver process, it should now receive the message. All we need to do is deserialize it, and apply it to the proper class:

schema_data = json.loads(raw)

for view, data in schema_data.items():
    cls = globals().get(view)
    cls.schema = data

I think something like this could allow you to scale without having to constantly add and remove views. But also allowing you to have multiple processes if you need.

#12

Thank you very much!
I will try it and let you know the results.

I agree that RabbitMQ is a great tool but it requires additional settings and needs to be installed in addition to Sanic application. I want Ax (my workflow application builder) to have option to run as standalone application. Independently of webservers, databases, external services etc.
I want users to run pip install ax and get fully working application. (Sanic helps greatly in achieving this).

Right now i use aiopubsub as pubsub tool, but it is one process only. I will try to find some other module and let you know.

#13

Another use case, for the record, against deprecate remove_route (I’m ok too about a workaround)
The server starts check if the db has been setup
If not it mounts the setup endpoint
else mounts the regular endpoints

If need setup, when the setup is finished, remove the setup endopoint and load the regular ones

We are building enduser apps with sanic where the setup process MUST be as user friendly as the regular processes

#14

Yes, I remember your concerns about this feature and I bring it to every discussion we have regarding the Router and its capabilities. We also understand this is an edge case and we’ll try our best to make it available - if not, try to help you with any workaround possible to have the same behavior to your end users (even if my own time is required to). Don’t worry :wink:

1 Like
#15

It is almost like there is a need for a “temporary” router before the setup processes completes. :thinking:

#16

I’ve seen things like this in webapps before and my general thought is that it’s maybe more prudent to restart the app once the new routes are in place so that they’re loaded along with any configs, because usually when the routes change, the configs do too - like database creds, perhaps backend static storage, etc.

#17

A remove_route would have been useful in my case.
I have a webapp in which dynamically created objects add routes (including web-socket routes) that point to their instance methods. When these objects are no longer required, the app needs to delete them, and remove associated routes. This would have been elegant with the remove-route feature. These objects are 'mini-webapps (actually mini-webapps and websocket-apps) which are dynamically created and destroyed.

I guess that since this is deprecated, I need to incorporate work-arounds (a little hacky.)