Skip to content
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

fix: Fixed swagger error. Fixes #8922 #9078

Merged
merged 2 commits into from
Jul 14, 2022
Merged

fix: Fixed swagger error. Fixes #8922 #9078

merged 2 commits into from
Jul 14, 2022

Conversation

JessieTeng89
Copy link
Contributor

Signed-off-by: Teng, Jessie yilin.teng@fmr.com

fix: fix swagger error. Fixes #8922

Signed-off-by: Teng, Jessie <yilin.teng@fmr.com>
@JessieTeng89
Copy link
Contributor Author

Hi @alexec
I created a PR to fix #8922, kindly help to review it if you are free, Thanks

Comment on lines +301 to +331
"google.protobuf.Any": {
"type": "object",
"properties": {
"type_url": {
"type": "string"
},
"value": {
"type": "string",
"format": "byte"
}
}
},
"grpc.gateway.runtime.Error": {
"type": "object",
"properties": {
"code": {
"type": "integer"
},
"details": {
"type": "array",
"items": {
"$ref": "#/definitions/google.protobuf.Any"
}
},
"error": {
"type": "string"
},
"message": {
"type": "string"
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall looks good,
I'm not sure if we need to redefine these here though.
These are defined in api/openapi-spec/swagger.json

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we got the definitions not found error when opening pkg/apiclient/_.primary.swagger.json, so I copied them from the file you mentioned

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the _.primary.swagger.json is an auxiliary file that helps to construct api/openapi-spec/swagger.json
We should just validate api/openapi-spec/swagger.json

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I only checked the swagger file for pkg/apiclient/_.primary.swagger.json before as the related issue is only to fix the error in it.
And I tried to open api/openapi-spec/swagger.json just now, there is no error.
Can you help to confirm if this PR is still needed or we can close the the issue and PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tczhao I saw this pull request is already approved, can we merge it or need some other actions from my side, kindly let me know if needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @tczhao @alexec
A gentle follow up with this PR, I am looking forward for your feedback 😄

@JessieTeng89
Copy link
Contributor Author

Hi @tczhao @alexec @sarabala1979
A gentle follow up with this PR, I am looking forward for your feedback

@JessieTeng89
Copy link
Contributor Author

@tczhao Thanks for your approval, can we merge this to master branch or need to ask someone else to do it?

@alexec alexec merged commit 2b92b1a into argoproj:master Jul 14, 2022
reddymh pushed a commit to reddymh/argo-workflows that referenced this pull request Jan 2, 2023
Signed-off-by: Teng, Jessie <yilin.teng@fmr.com>
Signed-off-by: Reddy <Rajshekar.Reddy@lowes.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Have errors for swagger.json when opening with https://editor.swagger.io
3 participants