unit icon indicating copy to clipboard operation
unit copied to clipboard

Remove GET method support from the application restart control API endpoint

Open javorszky opened this issue 1 year ago • 4 comments

#1445 will have added support for POST requests to restart applications alongside GET requests.

We should remove support for GET requests in Unit version 1.35 as it's semantically incorrect.

javorszky avatar Sep 27 '24 14:09 javorszky

NAK.

This could end up breaking a lot of peoples scripts.

We will just need to live with it...

ac000 avatar Sep 27 '24 14:09 ac000

NAK.

I'm not familiar with this, what does this mean in this context?

javorszky avatar Sep 27 '24 14:09 javorszky

No 4.

ac000 avatar Sep 27 '24 17:09 ac000

We will make this change. An effectful GET is a design flaw, and potentially a dangerous one. We'll do what we can to mitigate harm (graceful deprecation cycle, callouts on the blog, appropriate error logs during transition, and personal outreach to known users), but the longer we wait, the more painful this change will be.

The restart endpoint, today, can fail with an HTTP 500:

https://github.com/nginx/unit/blob/0e4342fa947fabde643c1482c70a3a3250756570/src/nxt_controller.c#L2426-L2434

Sophisticated operators in heavily automated environments should already be prepared to handle and alert on failure. Less automated operators will likely be manually running scripts / sending requests; they'll see appropriate error messages.

callahad avatar Sep 30 '24 15:09 callahad