airr-community / airr-standards

AIRR Community Data Standards
https://docs.airr-community.org
Creative Commons Attribution 4.0 International
35 stars 23 forks source link

Clarify API documentation regarding `size` query parameter #661

Closed bussec closed 5 months ago

bussec commented 1 year ago

There seem to be some inconsistencies regarding the documentation of the ADC API's size query parameter.

There are three locations in the ADC API docs that make statements about this parameter:

The docs state that size is "repository dependent", i.e., repos may choose size limits, even different ones for different endpoints. Those limits would be communicated to the user via the /info endpoint. However, neither the InfoObject in the AIRR Schema nor the service_info_object in the ADC schema contain any fields for this.

Furthermore, there is a max_size property returned by the /info endpoint, which gives the maximum size that a user can set without triggering an error, so this seems to be distinct of the use described above. Also, this seems to apply to all endpoints of a repository.

The docs should clarify whether size and max_size are distinct and whether or not an endpoint specific limit is communicated to the user and how.

bcorrie commented 5 months ago

This issue is a bit out of date, but I have done the following:

Hopefully that fixes things?

bcorrie commented 5 months ago

The docs state that size is "repository dependent", i.e., repos may choose size limits, even different ones for different endpoints.

No this is not correct - hopefully the docs don't say this any more 8-)

max_size is repository dependent. It is the same for all endpoints in a repository.

size is not a limit, it is a parameter passed into a query. It is an error in any query to ask for size > max_size objects in a query for a specific repository.

schristley commented 5 months ago

max_size is repository dependent. It is the same for all endpoints in a repository.

We do have an exception to this with the repertoire endpoint.

bcorrie commented 5 months ago

There is no way to communicate that with our current /info response, so I think the docs are fine as is? @schristley this would make the /repertoire endpoint not compliant, but it doesn't appear that would be a significant problem for the end user. Can we leave it at that? The only alternative would be to change the /info endpoint to have a per endpoint limit.

schristley commented 5 months ago

Yes, I know, I'm being annoying ;-D The repertoire endpoint did work that way initially but then it was annoying for the gateway to have to do multiple requests to get all of the repertoires, so we put in a little hack to return all. I don't want to change the /info endpoint just for that, and yeah I don't think it's a significant problem for end users. Is it worthwhile to add an additional disclaimer sentence, something like "For some endpoints, e.g. /repertoire, where the total number of records is not large, a repository may not implement max_size and instead return all of the data"? Or if you think that just confuses things more then the current docs are fine.

The other alternative is I change /repertoire back to be compliant, but I don't think anybody wants that! ;-D

bcorrie commented 5 months ago

Lets leave the docs as they are, and if there is an issue we can deal with it. I prefer the non-compliant VDJServer /repertoire endpoint 8-)

I will close this issue...