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

openapi: support content-negotiation via Accept for /workspace #210

Merged
merged 2 commits into from
Aug 14, 2022

Conversation

kba
Copy link
Member

@kba kba commented Jul 7, 2022

cf. OCR-D/zenhub#103

Depending on the Accept HTTP header, return either the JSON description of the workspace or the workspace itself.

@kba kba requested a review from joschrew July 7, 2022 10:55
Copy link
Contributor

@joschrew joschrew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks reasonable to me what you did here but I would suggest thinking about skipping the additional content types for put and delete. It is logical to me to offer that but usually there should be no need to download before deleting or updating. Except getting and downloading in one step. But if you program a client like that it is 1st unclear and 2nd not restfull in my eyes. And users could fetch the workspace before deleting or updating (and deleting with it) by themselfs. This way the API stays easier.
Additionally I would prefer to change the description of the get-request to something like: "Return Workspace or download OCRD-ZIP " to make it clearer what can be done. To non openapi-experts like me it could be easyier to get what can be done.
But I would go with the initial changes you made as well, so feel free to completely/partly skip my suggestions.

ps: I don't know if/how I can add my change-suggestions to this PR as just a suggestion and approve anyway so I skipped that.

@kba
Copy link
Member Author

kba commented Jul 7, 2022

ps: I don't know if/how I can add my change-suggestions to this PR as just a suggestion and approve anyway so I skipped that.

Feel free to change accordingly. For DELETE it really makes little sense, for PUT it might be, but I really don't care either way. In reality, I almost never inspect the response to POST/PUT/DELETE except for error messages.

You can directly append the PR by checking out the openapi-workspace-zip branch:

git fetch origin
git checkout -t origin/openapi-workspace-zip
# do changes, commit and push

@kba kba merged commit 04385bc into master Aug 14, 2022
@kba kba deleted the openapi-workspace-zip branch August 14, 2022 15:46
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.

2 participants