[rest] add DELETE method for pending dataset endpoint#3308
[rest] add DELETE method for pending dataset endpoint#3308raman325 wants to merge 2 commits intoopenthread:mainfrom
Conversation
Add support for DELETE on /node/dataset/pending to allow canceling a pending operational dataset (e.g. a scheduled channel migration) before it takes effect. The implementation clears the pending dataset by calling otDatasetSetPendingTlvs with a zero-length TLV buffer. Only the pending dataset supports DELETE; the active dataset endpoint continues to reject DELETE with 405. Also updates the OpenAPI spec to document the new method.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to delete the pending operational dataset via the REST API. It adds a DELETE method to the /node/dataset/pending endpoint in the OpenAPI specification and implements the corresponding logic in RestWebServer. The implementation clears the pending dataset by setting it with zero-length TLVs. I have no feedback to provide.
|
Downstream PR that consumes this endpoint: home-assistant/core#168441 (includes a fallback for firmware without DELETE support). |
From what I understand the delay is used to let the pending dataset propagate through the Thread network before it becomes active on the BR side. Not sure how well cancelling this process is supported: E.g. if we'd cancel shortly before it gets applied, a client might not receive the cancellation before the new configuration is applied (if cancellation is sent at all?). I fear this could make devices get stranded on a new but then cancelled configuration 🤔 @jwhui any thoughts on this? |
There may be good reasons why a user wants to cancel the operation, even if it may risk stranding nodes. It's hard to say. In this case, I would probably lean towards not having the network impose a limit and leave it to the client to decide whether to check the remaining delay of any existing pending dataset. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3308 +/- ##
===========================================
- Coverage 55.77% 35.75% -20.03%
===========================================
Files 87 145 +58
Lines 6890 17600 +10710
Branches 0 1462 +1462
===========================================
+ Hits 3843 6293 +2450
- Misses 3047 10962 +7915
- Partials 0 345 +345 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There are definitely good reasons. I guess what I wonder how high are the chances that nodes get stranded. And in any case, we should document the risks here (and clients probably should warn the user before triggering the delete). I did a bit of digging with the help of Claude, so take this with a grain of salt: There are two fundamental ways how it can go down: To me it seems the most likely outcome is a split Thread network. Or do I misunderstand something here? |
Summary
Add support for
DELETEon/node/dataset/pendingto allow canceling a pending operational dataset (e.g. a scheduled channel migration) before it takes effect.Context
While adding pending channel migration visibility to Home Assistant's OTBR integration (home-assistant/core#168441), we discovered that the REST API has no way to cancel a pending dataset. The
python-otbr-apilibrary already has adelete_pending_dataset()method that sendsDELETE /node/dataset/pending, but the server returns405 Method Not Allowedsince the endpoint only registers GET, PUT, and OPTIONS handlers.Note that the global CORS header (
Access-Control-Allow-Methods) already advertisesDELETEas a supported method, so OPTIONS preflight responses already indicate DELETE is allowed — the endpoint just wasn't handling it.The use case: when a user initiates a channel change via the OTBR, a pending dataset is created with a 5-minute delay. If the user made a mistake, there is currently no REST API method to cancel it — only the OpenThread CLI (
dataset clear) or D-Bus can do this.Changes
rest_web_server.cpp: Register aDeletehandler for the pending dataset endpoint. AddDeletePendingDataset()which clears the pending dataset by callingotDatasetSetPendingTlvswith a zero-length TLV buffer. AddkDeletecase to theDatasetdispatcher (only for pending; active dataset continues to reject DELETE with 405).rest_web_server.hpp: DeclareDeletePendingDataset.openapi.yaml: Document the new DELETE method.Testing
Tested against a live OTBR (official HA addon connected to a ZBT-2 coordinator) via the Home Assistant websocket API — confirmed the pending dataset is cleared and
get_pending_datasetreturnsNoneafter deletion.