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

Change API response schema to be a valid GeoJSON #100

Merged
merged 10 commits into from
Aug 9, 2021
Merged

Change API response schema to be a valid GeoJSON #100

merged 10 commits into from
Aug 9, 2021

Conversation

matthiasschaub
Copy link
Collaborator

@matthiasschaub matthiasschaub commented Jul 20, 2021 •
edited

Corresponding issue

Closes #46
Closes #88

Checklist

@matthiasschaub matthiasschaub force-pushed the api-response-schema branch 2 times, most recently from 914cef5 to 3fee9a8 Compare July 26, 2021 11:37
@matthiasschaub matthiasschaub marked this pull request as ready for review July 26, 2021 11:42
@matthiasschaub matthiasschaub changed the title Change API response to return a GeoJSON Change API response schema to be a valid GeoJSON Jul 26, 2021
@matthiasschaub matthiasschaub added the waiting for review This pull request urgently needs a code review label Jul 26, 2021
@matthiasschaub matthiasschaub added this to the Release 0.5.0 milestone Jul 28, 2021
joker234
joker234 requested changes Jul 29, 2021
View reviewed changes
Copy link
Member

@joker234 joker234 left a comment

Choose a reason for hiding this comment

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

Very good work, especially on the refactoring of the tests. Apart from the few comments I made, I think it would be good to redo all the vcr_cassettes. Some have to be deleted since you renamed tests and the User Agent changed for all requests (see #62).

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
workers/ohsome_quality_analyst/api.py Outdated Show resolved Hide resolved
workers/ohsome_quality_analyst/oqt.py Outdated Show resolved Hide resolved
workers/ohsome_quality_analyst/oqt.py Show resolved Hide resolved
workers/ohsome_quality_analyst/oqt.py Outdated Show resolved Hide resolved
workers/tests/integrationtests/test_api_report.py Outdated Show resolved Hide resolved
workers/tests/integrationtests/test_api_report_geojson_io.py Outdated Show resolved Hide resolved
@joker234 joker234 removed the waiting for review This pull request urgently needs a code review label Jul 29, 2021
@matthiasschaub
Copy link
Collaborator Author

Very good work, especially on the refactoring of the tests. Apart from the few comments I made, I think it would be good to redo all the vcr_cassettes. Some have to be deleted since you renamed tests and the User Agent changed for all requests (see #62).

Thanks! Good idea. I recreated the VCR cassettes and integrated you suggestions.
Could you take another look? I left the small commit messages as they are for the review, but will rebase after you viewed the changes again.

@matthiasschaub matthiasschaub force-pushed the api-response-schema branch 2 times, most recently from ab2c647 to 13fa61c Compare August 4, 2021 08:58
@joker234 joker234 added the enhancement New feature or request label Aug 4, 2021
joker234
joker234 previously approved these changes Aug 4, 2021
View reviewed changes
matthiasschaub and others added 10 commits August 5, 2021 12:26
@matthiasschaub @joker234
Change API response to return a GeoJSON
3d0d876
@matthiasschaub @joker234
Recreate VCR cassettes
6be9d31
@matthiasschaub @joker234
Add changes to CHANGELOG
e6495ab 
Reformating and rewording of the CHANGELOG
@matthiasschaub @joker234
Minor changes to incorporate review feedback
d2a191d 
Minor changes to logging message formatting

Add missing type hints

Minor changes to string concatenation syntax

Renamed variable to avoid overwriting existing var
@matthiasschaub @joker234
Recreate VCR cassettes
dfab2f2 
Recreate VCR cassette

Recreate VCR casssettes

Recreate VCR cassette
@joker234
urlencode parameters during tests
ce6a609
@joker234
recreate api_report_geojson_io VCR cassette
a677c81
@matthiasschaub @joker234
Fix bug due wron function parameters after rebase
0a9b0a1 
with main.
@matthiasschaub @joker234
Add request url parameter to response_template
93bef4e 
function. Rename url to request_url.
@matthiasschaub @joker234
Add tests for empty_api_response function
a267dd1
@matthiasschaub matthiasschaub merged commit 8f6677d into main Aug 9, 2021
@matthiasschaub matthiasschaub deleted the api-response-schema branch August 9, 2021 09:59
joker234 added a commit that referenced this pull request Aug 10, 2021
@joker234
add report (non-indicator) properties to GeoJSON output
1b1b040 
fixes bug introduced in #100
This was referenced Aug 10, 2021
Merged
Closed
joker234 added a commit that referenced this pull request Aug 10, 2021
@joker234
add report (non-indicator) properties to GeoJSON output
59d7918 
fixes bug introduced in #100
joker234 added a commit that referenced this pull request Aug 10, 2021
@joker234
add report (non-indicator) properties to GeoJSON output
063e04e 
fixes bug introduced in #100
@joker234 joker234 mentioned this pull request Aug 10, 2021
Merged
4 tasks
joker234 added a commit that referenced this pull request Aug 10, 2021
@joker234
add report (non-indicator) properties to GeoJSON output
965dc2b 
fixes bug introduced in #100
joker234 added a commit that referenced this pull request Aug 10, 2021
@joker234
add report (non-indicator) properties to GeoJSON output
6af6d2d 
fixes bug introduced in #100
joker234 added a commit that referenced this pull request Aug 11, 2021
@joker234
add report (non-indicator) properties to GeoJSON output
151c472 
fixes bug introduced in #100
matthiasschaub pushed a commit that referenced this pull request Aug 11, 2021
@joker234 @matthiasschaub
add report (non-indicator) properties to GeoJSON output
57dcdf4 
fixes bug introduced in #100
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joker234 joker234 joker234 approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
Release 0.5.0
2 participants
@matthiasschaub @joker234