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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

swagger tweaks: reordering and remove deprecated parameters #168

Merged
merged 3 commits into from
Jun 22, 2021

Conversation

tyrasd
Copy link
Member

@tyrasd tyrasd commented Mar 25, 2021

Description

A few changes to the swagger api playground:

  • reorder "tags" in specs:
    • for aggregation resources: count first, then length, area and perimeter, then users and last contributions
    • for data extraction: geometry, then elementsFullHistory/geometry, then contributions/geometry
    • no change for metadata 馃槈
    • reasoning: alphabetical sorting is not the most user friendly, IMHO. I propose the following sorting: simpler, more often used resources should come first, and more complex ones later
  • start phasing out deprecated types/keys/values parameters by not offering them in swagger anymore
    • reasoning: a) it makes it more evident that these parameters should not be used, b) we already have a few endpoint which do not support these old deprecated filters (e.g. all contributions endpoints), yet swagger still shows them as apparently valid options.
  • make time parameter for contribution endpoint optional (fall back to full time range if not specified??) TODO: check if this is a good idea and if it even works as intended

Checklist

@tyrasd tyrasd added documentation Improvements or additions to documentation brainstorming Idea for a potential new feature or adaption that still needs further discussion comments welcome Indicates that the creator of this issue/PR is open for early review comments labels Mar 25, 2021
@tyrasd tyrasd added this to the 1.5 milestone Mar 25, 2021
@tyrasd tyrasd added this to In progress in ohsome API general via automation Mar 25, 2021
@tyrasd tyrasd changed the title Swagger tweaks [suggestion] swagger tweaks Mar 25, 2021
@FabiKo117
Copy link
Contributor

I agree with both points: bringing a specific ordering in the speccs and further removing the deprecated types, keys, values parameters. This also reduces the code further.

What just came into my mind: Would it make sense to include a specific message (e.g. in a dedicated JSON field) in the response if the user used a deprecated parameter, endpoint, whatever to get this response and that he/she should check the docs and update this, as this specific request won't work anymore after the next major release? To better display this, we could add a specific section in the docs explaining which parameters, endpoints, etc. were deprecated in which version and from when they won't be available anymore.

@tyrasd
Copy link
Member Author

tyrasd commented Mar 26, 2021

Would it make sense to include a specific message (e.g. in a dedicated JSON field) in the response if the user used a deprecated parameter, endpoint, whatever to get this response and that he/she should check the docs and update this, as this specific request won't work anymore after the next major release?

That sounds like a good idea! 馃憤 At least for the JSON (aggregation) output format this should be possible without a problem. I'm not 100% sure about csv (maybe including it in the header comment would work?) and GeoJSON, though.

@tyrasd tyrasd removed comments welcome Indicates that the creator of this issue/PR is open for early review comments brainstorming Idea for a potential new feature or adaption that still needs further discussion labels Jun 22, 2021
by not displaying it in swagger anymore
@tyrasd tyrasd added the waiting for review This pull request needs a code review label Jun 22, 2021
ohsome API general automation moved this from In progress to Reviewer approved Jun 22, 2021
@tyrasd tyrasd changed the title [suggestion] swagger tweaks swagger tweaks: reordering and remove deprecated parameters Jun 22, 2021
@tyrasd tyrasd merged commit e5db39e into master Jun 22, 2021
@tyrasd tyrasd deleted the swagger-tweaks branch June 22, 2021 13:06
ohsome API general automation moved this from Reviewer approved to Done Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation waiting for review This pull request needs a code review
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants