-
Notifications
You must be signed in to change notification settings - Fork 167
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
dealing with api/v1 vs apis/extensions/v1beta1 is annoying since they are split between url and version #284
Comments
Right! Here's our similar code, also spliting on
|
I'd love to have multi-version in a single client too, so you can pass the
version as an option, but that can be step 2 ...
to be backwards compatible it could just blow up if the url has a path or
store the url path as `version_base` and then not override it when present
…On Thu, Jan 11, 2018 at 11:47 AM, Beni Cherniavsky-Paskin < ***@***.***> wrote:
Right! Here's our similar code, also spliting on /:
https://github.com/ManageIQ/manageiq-providers-openshift/
blob/93009fcb167/app/models/manageiq/providers/openshift/
container_manager_mixin.rb#L40-L52
1. Any idea if this can be backward compatible?
2. Does this interact with #208
<#208>? I this would be a
first step to make multi-version use by multiple Client instances a tiny
bit less annoying, and doesn't affect/depend on ideas for multi-version
with one instance...
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#284 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAsZzZqOe-Ziubam5DgdNHQThGTUfFzks5tJmU1gaJpZM4RZ3n7>
.
|
For anyone tackling this: the tests added in #457 are a good starting point. However, if it makes things easier, can have new keyword params for new behavior, supporting only one scheme.
|
url should be everything without the /api ... and version should be either
api/v1
or maybe justv1 / extensions/v1beta1
and then smartly detect if it should beapi
orapis
atm I'm using this which is kinda wonky
The text was updated successfully, but these errors were encountered: