Skip to content

Conversation

@juztas
Copy link

@juztas juztas commented Dec 9, 2025

This change replaces all inline HTTP response definitions in the OpenAPI YAML/JSON with pre-defined reusable response values under components.responses. All HTTP Responses are now referenced to the same set of structured responses (200, 304, 400, 401, 403, 404, 405, 422, 500).

The update also standardizes the list of HTTP status codes returned by each API. Even if a specific endpoint does not raise a specific erroc code (e.g. 404), the responses are still included for consistency. Some errors might be introduced by outside layer, like reverse proxies, gateways, or might be in future extensions of API call. This aligns the API with common industry patterns (AWS, GitHub, Stripe) where standard http codes and schemas are defined centrally, and not under the individual operation.

This refactor does not change behavior of the API implementation. Next step, would be to create a dedicated errors.md file and describe general error responses.

This change replaces all inline HTTP response definitions in the OpenAPI YAML/JSON with pre-defined reusable response values under `components.responses`. All HTTP Responses are now referenced to the same set of structured responses (200, 304, 400, 401, 403, 404, 405, 422, 500).

The update also standardizes the list of HTTP status codes returned by each API. Even if a specific endpoint does not raise a specific erroc code (e.g. 404), the responses are still included for consistency. Some errors might be introduced by outside layer, like reverse proxies, gateways, or might be in future extensions of API call. This aligns the API with common industry patterns (AWS, GitHub, Stripe) where standard http codes and schemas are defined centrally, and not under the individual operation.

This refactor does not change behavior of the API implementation. Next step, would be to create a dedicated errors.md file and describe general error responses.
@juztas
Copy link
Author

juztas commented Dec 9, 2025

@jmacauley @pmrich for your review.

@pmrich
Copy link
Contributor

pmrich commented Dec 16, 2025

I remember looking at these and thinking some of them were not appropriate for the given interface for the text documentation of the spec, but looking now this makes sense to me.

Should we also add 501 Not Implemented to the list as there is a fair bit, especially in filesystems, that is optional?

@juztas
Copy link
Author

juztas commented Dec 16, 2025

501 - I would say yes. Additionally, I also faced 504 on NERSC (during an incident). I could expand and include those two aswell.

@jmacauley
Copy link
Contributor

Agreed. If we are allowing some operations not to be implemented at a facility, then 501 makes sense.

@juztas
Copy link
Author

juztas commented Dec 16, 2025

Pull request updated to include 501, 503, 504.
I could also see a use of 503 - e.g. Site is in downtime and unable to process request; 504 - based on what I saw testing NERSC.
Python code changes pull request here: doe-iri/iri-facility-api-python#17 (all the rest codes were already merged in Py implementation)

@pmrich
Copy link
Contributor

pmrich commented Dec 16, 2025

I like adding 503. Provides a consistent way when a facility has to take downtime and it changed between when the user checked the resource status, and the resource actually went down. Or the user didn't check or the flow is using a fail and fall back strategy

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.

3 participants