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

Wrapped entity key error for Tags linked entity endpoint #130

Closed
DianaOlympos opened this issue Dec 13, 2023 · 4 comments · Fixed by #131
Closed

Wrapped entity key error for Tags linked entity endpoint #130

DianaOlympos opened this issue Dec 13, 2023 · 4 comments · Fixed by #131

Comments

@DianaOlympos
Copy link

The tags/{id}/{entity_type} endpoint is officially supported by pdpyras (see https://github.com/PagerDuty/pdpyras/blob/main/pdpyras.py#L198), but it is a paginated wrapped entity endpoint that does not respect the rules, as it uses the entity_type as its wrapper key. We get errors if we try to use iter_all/list_all on it.

Is there a solution, or should I just use the "raw" queries and do my loop pagination myself?

I would be happy to make a PR if an idea of what the solution should be is offered; right now, I am not sure of the best way, if any.

@Deconstrained
Copy link
Collaborator

Deconstrained commented Jan 5, 2024

Hello, and pardon the late reply! For which values of {entity_type} do the errors happen, and what are the errors?

The documentation for GET /tags/{id}/{entity_type} indicates that the entity wrapper name matches the last path node. If the API is not always responding with the documented schema, it is within reason to consider this a bug in the API itself. In that case, I'd be glad to create an issue report for it.

@Deconstrained
Copy link
Collaborator

Deconstrained commented Jan 5, 2024

I think I identified the issue (pdpyras.URLError). Thanks for bringing it to my attention. I can think of a few ways to make the client recognize the endpoint as an exception to the otherwise-global rule (which I overlooked last spring when I worked on v5.0.0) and the best way is an allowlist of canonical paths where the last part of the path can be a variable parameter albeit one that refers to a value in a fixed list of well-recognized entity types (as opposed to a separate but similar API endpoint with its own documentation for each {entity_type}).

In short, the idea behind an adequate fix is to make the guardrail that says "this endpoint wasn't meant for pagination" able to properly evaluate the path as referring to a collection and not an individual resource.

Edit: I have decided on a solution.

@DianaOlympos
Copy link
Author

Thank you so much! yeah, when I started diving, it definitely sounded like this was a "strange" endpoint from the data type pov.

If it helps, it was with users :)

@Deconstrained
Copy link
Collaborator

Deconstrained commented Jan 8, 2024

For the record: the solution decided upon was to expand the single canonical path into multiple paths as though they were separately documented for each explicit {entity_type}. For each of the resulting paths (which end in a constant string users, teams or escalation_policies vs. a variable parameter) the entity wrapping logic works as it should for any other path with no need for special new logic particular to this endpoint.

If new entity types are added to the tags API endpoint in question, support for them will need to be added by updating EXPAND_PATHS in the script that generates the global CANONICAL_PATHS list (and updating the declaration of said global accordingly).

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