feat: add v1beta1 api endpoints #70
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
added
lholmquist
reviewed
May 24, 2018
| routes: routes, | ||
| secrets: secrets, | ||
| services: services | ||
| buildconfigs, |
There was a problem hiding this comment.
heh, i actually have this changed in my branch where i'm switching to XO, so +1 :)
lholmquist
reviewed
May 24, 2018
lib/deployments.js
Outdated
| .apps}/namespaces/${clientConfig | ||
| .context.namespace}/deployments`; | ||
|
|
||
| console.log('find all at ', url); |
|
@lance i agree about the refactoring. in fact, there could be a lot more than just what you mentioned, that actually removes a lot of code since most, probably all, GET, POST, etc... are the same. But you are right, that shouldn't really be part of this PR, but a major version release after the refactor. |
|
speaking of endpoints, #42. i forgot about this one. i wonder during the refactor, if we should be changing the apis to look like this |
This commit adds the set of v1beta1 endpoint URLs documented here: https://docs.openshift.com/container-platform/3.7/rest_api/index.html The only functional endpoint at the moment is for deployments. All of the others simply expose the endpoint URL but do expose any functional behavior. I've added a `client.apis` object which maps to the the 3 semi-supported APIs at the moment. ```js client.apis.oapi // { version: config.apiVersion, baseUrl: client.apiUrl } client.apis.kube // { version: config.apiVersion, baseUrl: client.kubeUrl } client.apis.v1beta1 // function endpoints() returning array of endpoint URLs ``` Not sure if this is ready to merge or not. I think it's a good first step, and will give me what I need in nodeshift for nodeshift/nodeshift#228 but it almost feels like there's a major refactoring that could take place where the `lib` directory is split between the various supported APIs. For example: ```bash . └── lib ├── kube ├── oapi └── v1beta1 ``` To actually do all of this could be quite large, however. So maybe this commit is good for a minor version release, and the bigger changes can come later with a major version bump.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit adds the set of v1beta1 endpoint URLs documented here:
https://docs.openshift.com/container-platform/3.7/rest_api/index.html
The only functional endpoint at the moment is for deployments. All of
the others simply expose the endpoint URL but do expose any functional
behavior.
I've added a
client.apisobject which maps to the the 3 semi-supportedAPIs at the moment.
Not sure if this is ready to merge or not. I think it's a good first step,
and will give me what I need in nodeshift for nodeshift/nodeshift#228
but it almost feels like there's a major refactoring that could take place
where the
libdirectory is split between the various supported APIs.For example:
. └── lib ├── kube ├── oapi └── v1beta1To actually do all of this could be quite large, however. So maybe this
commit is good for a minor version release, and the bigger changes can
come later with a major version bump.
Connects to: #69