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

EAM API: clarify appinstances get behavior #319

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gainsley
Copy link
Collaborator

@gainsley gainsley commented Nov 6, 2024

What type of PR is this?

  • correction

What this PR does / why we need it:

Clarifies the behavior of the GET appinstances API if there are no matching instances to return. Implementors may consider returning 404 in this case, but I believe it is more correct to return 200 with an empty list, because:

  • 404 is also returned if the URL path is incorrect, which makes it hard to distinguish between no matches vs invalid URL path
  • Users (in particular tests) may intentionally be checking for the absence of an instance. Returning 404 in this case complicates verifying the absence of an instance.

Which issue(s) this PR fixes:

Special notes for reviewers:

Changelog input

Additional documentation

@gainsley gainsley force-pushed the clarify-appinstances-get-api branch from bfa7f17 to 416d441 Compare November 6, 2024 17:11
@gainsley gainsley changed the title clarify appinstances get behavior EAM API: clarify appinstances get behavior Nov 6, 2024
Copy link
Collaborator

@albertoec albertoec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @gainsley ,

I completely agree that for an empty list, the correct response should be a 200 HTTP status code with an empty array, rather than a 404. However, there are several other models (e.g., the response for GET /edge-cloud-zones) that currently behave this way, and this behavior isn’t explicitly specified.

I think we should decide whether we want to consistently specify the behavior for empty arrays in all responses.

What do you think?

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 this pull request may close these issues.

2 participants