This repository is currently being migrated. It's locked while the migration is in progress.
Introduce the same OpenAPI client as go-cli with small changes #225
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
We can get the creds via the storageos-api secret, and the endpoint via the node deployment. I'm having a play around with it now. Other than that, the implementation looks fine to me - caveat being that I've never looked at the go-cli code before |
The first commit in the chain of commits of this PR shows the go-api code after having simply moved it and the commits that follow it were always trying to simplify its implementation. I removed a lot of complexity it initially had. A good point made by Joe, one of the interfaces removed as part of that process could in fact be useful to mock the api methods. I haven't focused on unit tests to have notice this yet but it still stands. Any updates to the code are welcomed. |
nolancon
force-pushed
the
add-go-cli-api-client
branch
2 times, most recently
from
e482f1b to
8df0b18
Compare
nolancon
force-pushed
the
add-go-cli-api-client
branch
from
8df0b18 to
7b4bb7a
Compare
Some variables are hardcoded like username, password, apiendpoints, etc Removed a few structs with little use/purpose or that had too many dependencies in the go-cli project when trying to move into here Added a testing cmd `testapi` that calls a GetAllVolumes and prints the output
Use a chace for a single session (last one authenticated) which is still reusable when doing multiple requests.
Removed the api client implementation when no transport layer has been configured and would always return the same error indicating that. Now when creating a new API client, we need to pass the transport as a parameter thus we never fall into that scenario.
Very small changes like remove a method and update comments
Delete the codec struct and make each of its methods independent funcs so we dont have to pass around the struct instance and instead just call the coded method whenever needed
Moved a struct into another file with related content Removed lots of comments that add no context or value at all. We don't currently enforce any rules to include a go comment on every single method thus don't need to worry about filling any "quotes" with pointless "GetVolume returns a volume" comments
Merged the wrapper that adds session token caches with the one that adds extra useful methods like GetVolumeByName. Multiple comments updated to make responsabilities clearer. Removed the Transport interface. Without this interface we rely more on the embeded structs. Moved structs and errors related to openapi requests (volume not found, or request params for different endpoints) into the openapi package. Renamed the api wrapper that adds a retry when authentication failed to "reauth_client.go" Renamed structs representing the api clients from Transport into Client.
nolancon
force-pushed
the
add-go-cli-api-client
branch
from
7b4bb7a to
8ce6f85
Compare
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.
Introduced the same go-api implementation of our OpenAPI HTTP API and proceeded to make small changes to make it easier to use and understand.
As a result of such changes I introduced the following:
Currently using a test cmd
testapito make simpleGetAllVolumescalls and test this implementation.Still have to do: