Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KeyError when using rget/rput/rpost/list_all for EventOrchestrations API #96

Closed
bellmatt opened this issue Mar 16, 2023 · 4 comments · Fixed by #97
Closed

KeyError when using rget/rput/rpost/list_all for EventOrchestrations API #96

bellmatt opened this issue Mar 16, 2023 · 4 comments · Fixed by #97

Comments

@bellmatt
Copy link

There is an assumption in resource_envelope used for rget/rpost/rput and list_all functions that the name of the resource provided in the url path is the same as the name of the resource that comes back in the API response:
https://github.com/PagerDuty/pdpyras/blob/main/pdpyras.py#L127
https://github.com/PagerDuty/pdpyras/blob/main/pdpyras.py#L1138

However this doesn't always seem to be the case. For the event orchestrations APIs, the url path is /event_orchestrations and the response contains "orchestrations" or "orchestration_path" for the router/unrouted/service APIs
https://developer.pagerduty.com/api-reference/7ba0fe7bdb26a-list-event-orchestrations

so the result of list_all is a KeyError at pdpyras.py:1187
KeyError: 'event_orchestrations'

@Deconstrained
Copy link
Collaborator

@Deconstrained
Copy link
Collaborator

Keeping this open; there may be a compromise that does not require an abstraction layer for resource types (which is not the intent of design) but enables the convenience methods to be used with the newer endpoints. With some of the newer APIs, the envelope name is the only major departure from the conventions that were once universal. Such APIs could be accommodated without cluttering the codebase with special exceptions and workarounds.

@Deconstrained
Copy link
Collaborator

Deconstrained commented Mar 23, 2023

This may take a while and in the end I might decide on leaving this as-is. There are a lot of endpoints I need to review in order to decide on a design for a whole new level of abstraction to handle non-conformal APIs, and even then, future APIs might break assumptions that are safe to make for all the current APIs.

Some endpoints like POST /schedules/{id}/overrides and POST /business_services/{id}/subscribers are practically beyond redemption, i.e. because they use inconsistent entity wrapping between the response and request bodies, and it will require extremely specialized logic to support them. It's just a matter of finding a way of doing this that doesn't require completely rewriting how entity wrapping is currently implemented and keeps the specialized logic in one place versus littered throughout the codebase.

@Deconstrained
Copy link
Collaborator

Deconstrained commented Apr 4, 2023

It's coming along nicely. It's going to be a new major release. All current endpoints will be supported, although entity wrapping won't always be enabled. I'm developing a rubric for determining when it is enabled based on the endpoint. This won't completely eliminate the need to check the documentation of a given API before using it, but it will do away with the extreme rigidity and usability landmine of having supported vs non-conformal endpoints with respect to entity wrapping.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants