Bug when posting to /schedules/{id}/overrides
sl-alexandrenerat opened this issue · 4 comments
Hi,
When posting to /schedules/{id}/overrides, logic defined here (https://github.com/PagerDuty/pdpyras/blob/main/pdpyras.py#L130):
if is_index and http_method=='get' or multi_put:
# Plural resource name, for index action (GET /<resource>), or for
# multi-update (PUT /<resource>). In both cases, the response
# (former) or request (latter) body is {<resource>:[<objects>]}
envelope_name = resource
else:
# Individual resource create/read/update
# Body = {<singular-resource-type>: {<object>}}
envelope_name = envelope_name_single
leads to the latter case in which we are updating an individual resource which is not the case. Specifically:
- is_index is true because url has 3 parts
- http_method is POST, so we fail in the else part
As a consequence, when performing session.rpost('/schedules/{id}/overrides', json=overrides), overrides provided are being encapsulated as a "override" property of an object.
Workaround is to pass overrides as:
overrides = {
overrides= [ ... ],
override = ''
}
Then, as override property is present, the whole object is being sent.
Hi @sl-alexandrenerat I believe I am having the same issue when attempting to post to /incidents/{uid}/snooze I wonder if I am encountering the same issue.
Curl requests to api.pagerduty.com for that path work correctly. However, when I try through pdpyras I get this
POST /incidents/#####/snooze: API responded with non-success status (400): {"error":{"message":"Invalid Input Provided","code":2001}}How did you troubleshoot this issue and identify where the problem was occurring? I am stuck determining how to move forward with gathering more information on what is going.
@jwrobb I'm not sure it's the same bug actually as your api call doesn't post a list designated with a plural property.
Anyway to find out which was the problem I added debug to requests library (here https://gist.github.com/Daenyth/b57f8522b388e66fcf3b?permalink_comment_id=3194372#gistcomment-3194372) and also used a debugger to step into my call.
Hello @sl-alexandrenerat,
Thank you for pointing this out. This is a counter-pattern in the API that I hadn't noticed before. It breaks rpost in this use case, but it doesn't need to.
An ideal fix would be defining a static list of endpoints at the top similar to VALID_MULTI_UPDATE_PATHS, i.e. VALID_MULTI_CREATE_PATHS = [('schedules', '{id}', 'overrides')] and then conjoin a similar thing to multi_put to the condition, i.e.
multi_post = http_method == 'post' and nodes in VALID_MULTI_CREATE_PATHS
...
if is_index and http_method=='get' or multi_put or multi_post:
@jwrobb I recommend not using rpost for snoozing an incident because the endpoint isn't supported by the r* methods.
If you had been using regular old post instead, then it would be worth examining the value of the json argument and ensuring that it's a dictionary that looks something like {"duration": 86400}
Upon further review, it looks like POST /schedules/{id}/overrides departs from API norms to an extent that makes it irreconcilable with the design of the resource_envelope decorator. This is because in the response, there is no resource envelope.
Per the API docs for POST /schedules/{id}/overrides, the root level of the response body is an array of objects, versus an object with a resource envelope property that is itself either an array or an object. No resource envelope means there's nothing to extract from the response body.
As a workaround, one could use session.jpost('/schedules/PSCHEDID/overrides', json={'overrides': [...]}) similarly to rpost; the return value is the array of created overrides. The only thing you're missing out on by doing this is being able to send just the array of overrides [...] as the json keyword argument.
The only way to make rpost work with the create overrides endpoint is making a super special exception for just this one particular API where it returns the response body in full, but that is treading into the territory of "making an API for the API" which goes against the design philosophy of this API client.
If there is another API that has this same behavior and it looks like this will become a trend (similar to multi-update), then it would be more more reasonable for rpost/resource_envelope to support such endpoints.