QutEcoacoustics/baw-server

`/status` endpoint does not follow the "Standard Response Format" found in API spec

hudson-newey opened this issue · 3 comments

Currently, the api returns the /status endpoint returns an object that doesn't follow the response format outlined in the baw-api "Standard Response Format".

Current Response Format:

{
	"status": "good" | "bad",
	"timed_out": boolean,
	"database": boolean,
	"redis": "PONG" | "",
	"storage": "No audio recording storage directories are available." | `${number} audio recording storage directory available.`,
	"upload": "Alive" | "Dead"
}

This is a problem since it doesn't have the meta and data properties expected from the spec structure. This means that this specific endpoint requires special client-side service conditions to work (since our abstract-service expects the content to be under the data property).

Expected response:

{
	"meta": {
		"status": 200,
		"message": "OK"
	},
	"data": {
		"status": "good" | "bad",
		"timed_out": boolean,
		"database": boolean,
		"redis": "PONG" | "",
		"storage": "1 audio recording storage directory available." | `${number} audio recording storage directory available.`,
		"upload": "Alive" | "Dead"
	}
}

Additionally, we might want to change status, redis, and storage properties to boolean types. This would make the object more readable.
If we want to retain information such as the number of storage directories available, we could emit a sub-model similar to the meta property (where we have an object with status and message properties).

We might also consider changing the timed_out key to something like server_connected so it isn't a negated property. At the moment redis = true, database = true, indicates that the connection is healthy, but timed_out = true means the connection is unhealthy. By changing it to sever_connected (or similar) it makes it easier to read since a true property is good, while a false property is bad.


I'm expecting this issue to turn into a discussion, so any feedback would be appreciated.

The API originally didn't follow the standard format because

a) back then we didn't have a standard
and mostly b) it was designed for the simplest possible parsing

As for the fields, most have the ability to include some kind of error message. They're not Boolean fields. In fact even the status field (which is Boolean) was made a string so it could be unambiguously matched with a simple grep.

I'm not opposed to changes, but they would be breaking.

For the fields we could make them unions boolean | string or each with the possibility of producing an error message could be of the type IResult { error: string?, alive: boolean }.

The API originally didn't follow the standard format because

That makes sense. Since it's a working feature and sounds like it will generate a lot of breaking changes, I'd like to put this on the backlog as a low-priority / stretch goal.

For the fields we could make them unions boolean | string or each with the possibility of producing an error message could be of the type IResult { error: string?, alive: boolean }.

Out of the two options, I'd prefer the overloading of types because:
a. It might might maintain backward compatibility with some scripts
b. I'm a bit hesitant about the IResult { error: string?, alive: boolean } structure as the error?: string is not in convention with our error spec

I'm a bit hesitant about the IResult { error: string?, alive: boolean } structure as the error?: string is not in convention with our error spec

Incorrect. The error spec for a start is singular (we have multiple possible error messages). Secondly, the error information is always for a request that has failed. A status request can successfully report the statuses and error messages of many components, without the status request itself failing.