-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: add client for the orchestration API [IDE-201] #20
Conversation
ac02ab5
to
5c0655b
Compare
2206105
to
c572f71
Compare
|
||
orgUUID := uuid.MustParse("e7ea34c9-de0f-422c-bf2c-4654c2e2da90") | ||
id := uuid.New() | ||
response, err := client.CreateScanWorkspaceJobForUserWithApplicationVndAPIPlusJSONBodyWithResponse( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's quite a long function/endpoint name - any chance to change it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is pretty long but as far as I can read from https://github.com/deepmap/oapi-codegen/blob/master/README.md#extensions we could only maybe change the name of the function through the spec.
There are some other things that I don't like about this generated code, one of which is that there are some untyped structs that make it awkward to use. I think what we could do is wrap these functions into some syntactic sugar functions when we get to using them. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd try to avoid working around generated code. If we want to do that, we should have our own generator that works around this. If the endpoint name is good enough, I guess we should just live with it.
Similar to #17, it uses
oapi-codegen
to generate the client for the orchestration API. This PR also changes the bash script to a python script.There will be two APIs we need to use so I only added tests for the two of them.