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.

Sign up for GitHub

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

Jump to bottom

extending data extraction endpoints with /contributions #34

Merged
merged 48 commits into from
Nov 4, 2020
Merged

extending data extraction endpoints with /contributions #34

merged 48 commits into from
Nov 4, 2020

Conversation

FabiKo117
Copy link
Contributor

@FabiKo117 FabiKo117 commented Aug 25, 2020 •
edited
Loading

This branch implements the first 2 points raised in issue #23, which are:

  • /contributions/geometry/ for the modifications as GeoJSON

  • /contributions/latest/geometry/ for the "latest" changes only, as GeoJSON

  • same endpoints also for /bbox and /centroid respectively

  • add sphinx docs for the new endpoint + examples

For further rationale, please look at the discussions in PR #18.

@FabiKo117 FabiKo117 changed the title extending data extraction endpoints with /contributions WIP:extending data extraction endpoints with /contributions Aug 25, 2020
@FabiKo117 FabiKo117 mentioned this pull request Aug 25, 2020
Closed
@FabiKo117 FabiKo117 added this to the 1.1 milestone Aug 25, 2020
@tyrasd tyrasd modified the milestones: 1.1, 1.2 Sep 2, 2020
@joker234 joker234 assigned joker234 and unassigned joker234 Sep 15, 2020
@joker234 joker234 added this to In progress in ohsome API general via automation Sep 15, 2020
@FabiKo117 FabiKo117 force-pushed the add-contributions-endpoint branch 2 times, most recently from 34bec81 to 03a1755 Compare September 17, 2020 15:39
@FabiKo117 FabiKo117 changed the title WIP:extending data extraction endpoints with /contributions extending data extraction endpoints with /contributions Sep 29, 2020
tyrasd
tyrasd previously requested changes Oct 2, 2020
View reviewed changes
Copy link
Member

@tyrasd tyrasd left a comment

Choose a reason for hiding this comment

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

I'm not done looking though the whole code yet, but I just realized one thing that I would really want to advice against: IMHO we should not add support for the old and deprecated types/keys/values parameters in this new endpoint is. Or is it unreasonably hard to drop the parameters for these resources?

src/main/java/org/heigit/ohsome/ohsomeapi/config/SwaggerConfig.java Outdated Show resolved Hide resolved
src/main/java/org/heigit/ohsome/ohsomeapi/config/SwaggerConfig.java Show resolved Hide resolved
ohsome API general automation moved this from In progress to Review in progress Oct 2, 2020
@FabiKo117
Copy link
Contributor Author

FabiKo117 commented Oct 2, 2020 •
edited
Loading

I'm not done looking though the whole code yet, but I just realized one thing that I would really want to advice against: IMHO we should not add support for the old and deprecated types/keys/values parameters in this new endpoint is. Or is it unreasonably hard to drop the parameters for these resources?

I did not specifically add a support for it. So I'm just using the same framework that already exists in terms of parameter processing. What we could do is remove types/keys/values from the list of allowed parameters for this resource, so the user would get an error message when trying to use one of them in a request to /contributions

@tyrasd
Copy link
Member

tyrasd commented Oct 2, 2020

What we could do is remove types/keys/values from the list of allowed parameters for this resource, so the user would get an error message when trying to use one of them in a request to /contributions

sounds good 👍 . additionally, I'd say that in swagger and the documentation the parameters should not be suggested to the user if possible.

@FabiKo117
Copy link
Contributor Author

What we could do is remove types/keys/values from the list of allowed parameters for this resource, so the user would get an error message when trying to use one of them in a request to /contributions

sounds good 👍 . additionally, I'd say that in swagger and the documentation the parameters should not be suggested to the user if possible.

I can remove them from swagger completely yes. The docs are another topic, which is actually still not existant for the /contributions endpoint. Need to add those still.

@FabiKo117 FabiKo117 changed the title extending data extraction endpoints with /contributions WIP: extending data extraction endpoints with /contributions Oct 5, 2020
This was linked to issues Oct 27, 2020
Adding rest error codes to documentation #64
Closed
Enhancing docs with description on parameters returned in the response #58
Closed
@tyrasd tyrasd dismissed their stale review November 2, 2020 09:34

should be fixed by now

ohsome API general automation moved this from Review in progress to Reviewer approved Nov 2, 2020
@FabiKo117 FabiKo117 changed the title WIP: extending data extraction endpoints with /contributions extending data extraction endpoints with /contributions Nov 2, 2020
@tyrasd tyrasd self-requested a review November 4, 2020 12:05
FabiKo117 and others added 24 commits November 4, 2020 17:40
@FabiKo117
adding tests on deletions
e541bea 
to check if a proper empty geometry is returned
@FabiKo117
changing geometry creation for deletions
ad27bb7 
to incorporate the clipping flag
@FabiKo117
removing unnecessary code
4560761 
to handle creations, as they can also be included easier in the for-loop
@FabiKo117
minor code quality and comment changes
cba009b 
to apply a better code flow
@FabiKo117
adding test on /contributions/latest
f51512f
@FabiKo117
refactoring of data-extraction
a9e1179 
making DataRequestExecutor unstatic
modifying javadoc comments accordingly
@FabiKo117
adding junit tests for /contributions get requests
6850c47
@FabiKo117
adding junit test for /contributions post requests
1feb38e
@FabiKo117
adding junit test for /contributions/latest post request
84bed51
@FabiKo117
modifying spec names in swagger
0f2de70 
to keep a specific ordering of the sites through adding numbers in front
@FabiKo117
modifying valid parameters for /contributions
d818a68 
removing types, keys, values from list of valid params as they are deprecated
@FabiKo117
modifying description for contributions in swagger
0989912
@FabiKo117
modifying swagger for /contributions
a10dce2 
removing types, keys, values parameters from swagger
@FabiKo117
adding junit test
2621afa
@FabiKo117
adding base docs for /contributions
82ffeb4 
adding a new section for the new endpoint
@FabiKo117
adding the /contributions/latest endpoint to docs
e53251e
@FabiKo117
add examples (curl, python, R) for contributions and contributions/la…
2bae972 
…test endpoint
@FabiKo117
contributions endpoint corrections
89d0ea0
@tyrasd @FabiKo117
swagger: make resource specs sortable
5467cfc
@FabiKo117
Adding parameters descriptions, solves #58
7f1009f 
adding http response status codes, solves #64
@FabiKo117
extending parameter descriptions in docs
b1e82dd 
adapting their grouping
modifying some descriptions
@FabiKo117
minor writing-style fixes in docs
f50dce2 
for HTTP status codes
@FabiKo117
Revert "minor writing-style fixes in docs"
bb6be21 
This reverts commit 1d909b5.
@FabiKo117
minor writing-style fixes in docs
d8f4c5f 
for HTTP response codes
@FabiKo117 FabiKo117 merged commit 2ab129c into master Nov 4, 2020
ohsome API general automation moved this from Reviewer approved to Done Nov 4, 2020
@FabiKo117 FabiKo117 deleted the add-contributions-endpoint branch November 4, 2020 16:57
@tyrasd tyrasd added this to In progress in contributions endpoints via automation Mar 19, 2021
@tyrasd tyrasd moved this from In progress to Done in contributions endpoints Mar 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tyrasd tyrasd tyrasd approved these changes

@bonaparten bonaparten bonaparten approved these changes

Assignees
No one assigned
Labels
None yet
Projects
contributions endpoints
Done
ohsome API general
  
Done
Milestone
1.2
4 participants
@FabiKo117 @tyrasd @bonaparten @joker234