-
Notifications
You must be signed in to change notification settings - Fork 3
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
Swagger api calls #17
Conversation
Does anyone know how to change the diffbase? Not sure if we can do this: |
src/backend/specifications.tsx
Outdated
@@ -1,5 +1,6 @@ | |||
import { Specification } from 'model/Specification'; | |||
import { BuildStatus } from 'model/SDK'; | |||
import { isUndefined } from 'util'; |
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.
@darvid7 Why do you need this? It's a deprecated method for one thing.
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.
Ty for pointing that out, idk webstorm just prompted me to use it :P
src/backend/specifications.tsx
Outdated
@@ -42,6 +53,11 @@ const specifications: Specification[] = [ | |||
* @return {Specification | undefined} - returns the Specification with the matching id if it exists | |||
*/ | |||
export function getSpecificationById(id: number): Specification | undefined { | |||
// TODO: Remove this later. | |||
if (isUndefined(id)) { |
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.
Why do we get the last item if we don't specify an id? The parameter doesn't allow for undefined values.
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.
When trying to implement it was always undefined so I just added it to show it can work.
Also the type being Specification | undefined, doesn't that allow it to be undefined?
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.
No as in id is never undefined (as specified by the parameter type) so why account for id
being undefined if the function explicitly says that id will only ever be a number (i.e. never be undefined)
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 is true, idk what I was thinking. Thanks for pointing it out!
src/client/sdkGeneration.tsx
Outdated
console.log('generateSdk'); | ||
let body = { swaggerUrl: spec['path'] }; | ||
console.log(body); | ||
return fetch(SWAGGER_CODEGEN_ENDPOINT + 'python', { |
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.
Can we change this to use await
?
- Change .then()s to
await
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.
Does this mean we need to nest them somehow because there are two .thens()?
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.
Figured out how to :) although if you know what .json() does pls lmk
src/client/sdkGeneration.tsx
Outdated
if ('type' in json && json['type'] == 'error') { | ||
return BAD_SPECIFICATION; | ||
} | ||
return json['link']; |
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.
Please use json.type
and json.link
rather than json['link']
- Change
['xx']
syntax to.xx
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.
Didn't know this was a thing, ty
src/client/sdkGeneration.tsx
Outdated
import fetch from 'node-fetch'; | ||
|
||
const SWAGGER_CODEGEN_ENDPOINT = 'http://generator.swagger.io/api/gen/clients/'; | ||
const BAD_SPECIFICATION = 'Error: Bad specification'; | ||
|
||
export async function generateSdk(spec: Specification): Promise<string> { |
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'm surprised this passed the checker:types
test. By the looks of things, generateSdk
doesn't return Promise<string>
anymore. Can we make this something less wrong for now? (E.g. Promise<any>
or something similar?)
- Change the return type of
generateSdk
src/client/sdkGeneration.tsx
Outdated
// TODO: Get it working for .yaml files like below. | ||
// 'https:/OAI/OpenAPI-Specification/blob/master/examples/v2.0/yaml/uber.yaml' } | ||
console.log('generateSdk'); | ||
let body = { swaggerUrl: spec['path'] }; |
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.
spec.path
rather than spec['path']
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.
Done
@PatrickShaw Please take another look, made all changes except the getting last item if an id is not specified. |
Wasn't any point having it since id is never undefined
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 think this LGTM now 👍
src/backend/specifications.tsx
Outdated
@@ -42,6 +53,11 @@ const specifications: Specification[] = [ | |||
* @return {Specification | undefined} - returns the Specification with the matching id if it exists | |||
*/ | |||
export function getSpecificationById(id: number): Specification | undefined { | |||
// TODO: Remove this later. | |||
if (isUndefined(id)) { |
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.
No as in id is never undefined (as specified by the parameter type) so why account for id
being undefined if the function explicitly says that id will only ever be a number (i.e. never be undefined)
General Updates Just going to merge it now cuz I forgot to merge it before #17 and idk what is going to happen to the git tree now :)
Add Swagger Codegen API call