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

/taxon API endpoint returns 404 if IRI not found, which is not backwards compatible #377

Open
hlapp opened this issue Jan 28, 2021 · 5 comments

Comments

@hlapp
Copy link
Member

hlapp commented Jan 28, 2021

curl -D - -X GET "https://kb.phenoscape.org/api/taxon?iri=foo" -H "accept: application/json"
HTTP/1.1 404 Not Found
Date: Thu, 28 Jan 2021 00:18:53 GMT
Server: akka-http/10.1.11
Content-Type: text/plain; charset=UTF-8
Content-Length: 42

The requested resource could not be found.

Arguably this is the correct behavior, but it is strictly backwards incompatible. With an unversioned API this essentially renders all current versions of clients (such as RPhenoscape) inoperable.

We really can't do backwards incompatible changes without first versioning the API. It wreaks havoc. Can we please revert this endpoint to how it originally behaved.

@hlapp
Copy link
Member Author

hlapp commented Jan 28, 2021

See phenoscape/rphenoscape#151 for how it showed up. This is currently breaking the tests for Rphenoscape.

@balhoff
Copy link
Member

balhoff commented Jan 28, 2021

I acknowledge that we haven't established a versioning system, but I would like to point out that you requested this change as a bug fix: #194 😄

@hlapp
Copy link
Member Author

hlapp commented Jan 28, 2021

you requested this change as a bug fix: #194 😄

Indeed I did 😀 The problem is the Rphenoscape code is working around the behavior of the API, right or wrong, in various places, and understanding a priori which changes if implemented would break backwards compatibility is hard.

Generally speaking, for obvious reasons any change from a success code to a failure code returned for the same input should be considered backwards incompatible, whether this is mentioned in the issue or not.

@hlapp
Copy link
Member Author

hlapp commented Jan 28, 2021

BTW Rphenoscape identifies itself in the header (as User-Agent, for example r-curl/4.3 httr/1.4.1 rphenoscape/0.2.15), including version. (You should see this in your logs.) Short of versioning (which I know is a big undertaking) you could use this information to prevent breaking it. For example, any version 0.2.x and lower would be broken with this change.

@balhoff
Copy link
Member

balhoff commented Feb 24, 2021

Rolled back by #384.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants