Add the possibility to specify multiple parameter arguments per type

Question

Add the possibility to specify multiple parameter arguments per type

reinhapa opened this issue 6 years ago · 7 comments

It would be great to support multiple parameter arguments per argument so it would be possible to select for example all jdk types for a specified amount of os like so:

https://api.adoptopenjdk.net/v2/latestAssets/releases/openjdk8?openjdk_impl=hotspot&os=linux&os=windows

Same could be used to select different arch, rekease, type and so on

Answer 1 · 2019-05-06T05:43:50.000Z

I've got a PR nearly ready for this, but in order to support multiple release query values I would need to make a decision related to #113.

tl;dr: due to the way the release query affects the response format, it would be non-trivial to handle multiple release values in a way that would be clear to users and not break existing functionality.

A possible workaround for now would be to disallow multiple values for release queries. This should prevent any breaking changes being introduced, and leave that particular issue to be addressed later.
(CC @karianna : Thoughts on this workaround?)

Answer 2 · 2019-05-06T06:22:27.000Z

@ParkerM @karianna as a consumer it makes more sense to filter based on os as we need have to support only those. A filtering on other parts could be done in a later version...

Answer 3 · 2019-05-06T16:43:18.000Z

I've got a PR nearly ready for this, but in order to support multiple release query values I would need to make a decision related to #113.

tl;dr: due to the way the release query affects the response format, it would be non-trivial to handle multiple release values in a way that would be clear to users and not break existing functionality.

A possible workaround for now would be to disallow multiple values for release queries. This should prevent any breaking changes being introduced, and leave that particular issue to be addressed later.
(CC @karianna : Thoughts on this workaround?)

I think if we disallow then we'll introduce unexpected behaviour for those who are already relying on the (admittedly bad) behaviour, so it would have to be a v3 patch....

Answer 4 · 2019-05-07T01:14:41.000Z

I think if we disallow then we'll introduce unexpected behaviour for those who are already relying on the (admittedly bad) behaviour, so it would have to be a v3 patch....

By that I meant adding multi-value query support for all existing fields except for release, so the API maintains its current behavior of returning a single object for ?release=latest and an array for ?release={releaseName}.

I receive a 504 Gateway Timeout after 30 seconds when multi-value query params are supplied in the current production v2 API. This could be modified to respond with a proper 400 Bad Request response when a multi-value release query is detected. This would presumably not break any workflows, and incidentally reduce server strain that may be caused by those timeouts.

See:
200: ?release=jdk8u212-b03_openj9-0.14.0
200: ?release=jdk8u202-b08
504: ?release=jdk8u212-b03_openj9-0.14.0&release=jdk8u202-b08

Answer 5 · 2019-05-07T08:59:30.000Z

@johnoliver Thoughts? I'm OK with this.

Answer 6 · 2019-05-07T09:37:14.000Z

Yeah I am fine with having release special cased for the time being until we get out v3 api and come up with something more consistent.

Answer 7 · 2019-05-07T23:24:21.000Z

Cool, I'll continue in that direction and should have a PR ready sometime in the next day or two 👍.