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

Updates to ICOB API's including support for versioning #343

Closed
wants to merge 9 commits into from

Conversation

ianco
Copy link
Contributor

@ianco ianco commented Sep 6, 2019

Work is in progress on this topic but opening the PR in Draft mode for visibility and feedback.

General cleanup of the v2 API's.

Support for multiple API versions using middleware = will detect version in an Accept header or in the URL and will map to the appropriate version of the service. Includes support for things like "default", "latest" or whatever other synonyms we want to support.

The underlying API URL's use the version in the url (such as "/api/v2") however we can hide this in the URL's that we expose.

Signed-off-by: Ian Costanzo <ian@anon-solutions.ca>
Signed-off-by: Ian Costanzo <ian@anon-solutions.ca>
@ianco
Copy link
Contributor Author

ianco commented Sep 6, 2019

@andrewwhitehead
Copy link
Contributor

If it's going to use the Accept header, I think the code should split it on commas first, since the header can have multiple values. I'm not sure if the space in the given examples is valid, as in application/orgbook.bc.api+json; version=v2.

Signed-off-by: Ian Costanzo <ian@anon-solutions.ca>
@ianco
Copy link
Contributor Author

ianco commented Sep 13, 2019

Taking this out of draft for whoever want to work on this next

@ianco ianco marked this pull request as ready for review September 13, 2019 17:50
Copy link
Member

@esune esune left a comment

Choose a reason for hiding this comment

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

I had a look at this before I got pulled into something else. Writing this thought down so that I/we don't forget: I am not sure we need to support old API versions for ICOB since it has not yet been released for production and I would therefore be inclined towards cleaning everything up and having a "fresh start" using - if possible - the standard versioning schemes supported by drf out of the box for simplicity.

Thoughts?

Signed-off-by: Ian Costanzo <ian@anon-solutions.ca>
Signed-off-by: Ian Costanzo <ian@anon-solutions.ca>
Signed-off-by: Ian Costanzo <ian@anon-solutions.ca>
@codecov
Copy link

codecov bot commented Oct 30, 2019

Codecov Report

Merging #343 into master will increase coverage by 0.94%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master    #343      +/-   ##
=========================================
+ Coverage   59.45%   60.4%   +0.94%     
=========================================
  Files          91      97       +6     
  Lines        2765    2912     +147     
=========================================
+ Hits         1644    1759     +115     
- Misses       1121    1153      +32

@esune
Copy link
Member

esune commented Oct 30, 2019

PR has become stale, will reopen when ready to move forward.

@esune esune closed this Oct 30, 2019
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.

3 participants