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

Fixing missing swagger spec for apis/extensions #16690

Merged
merged 2 commits into from
Nov 3, 2015

Conversation

nikhiljindal
Copy link
Contributor

Fixes #16687

The only difference between /apis/extensions and other paths (such as /apis) was "/" at the end. I tried removing it and that fixed the issue :)

cc @bgrant0607 @caesarxuchao @lavalamp

@k8s-github-robot
Copy link

Labelling this PR as size/M

@k8s-github-robot k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 2, 2015
@@ -672,7 +672,7 @@ func (m *Master) init(c *Config) {
Versions: expAPIVersions,
PreferredVersion: unversioned.GroupVersion{GroupVersion: storageVersion, Version: apiutil.GetVersion(storageVersion)},
}
apiserver.AddGroupWebService(m.handlerContainer, c.APIGroupPrefix+"/"+latest.GroupOrDie("extensions").Group+"/", group)
apiserver.AddGroupWebService(m.handlerContainer, c.APIGroupPrefix+"/"+latest.GroupOrDie("extensions").Group, group)
Copy link
Member

Choose a reason for hiding this comment

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

@nikhiljindal, this seems unrelated to /swaggerapi/...? This should only affect /apis/extensions/ ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

/swaggerapi is auto generated.
/swaggerapi/apis/extensions/ was returning ApiDeclaration not found. This change fixes that

Copy link
Member

Choose a reason for hiding this comment

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

I see. Maybe go-restful requires there's no trailing "/". Could you double check /apis/extensions/ and /apis/extensions still work? Because that's the paths directly affected by this line. Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

@nikhiljindal, Could you double check /apis/extensions/ and /apis/extensions still work? Because that's the paths directly affected by this line. Thanks.

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 Verified that both of them work

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! LGTM.

@bgrant0607
Copy link
Member

Why was the "/" there?

@nikhiljindal
Copy link
Contributor Author

@bgrant0607 I dont think that there was a specific reason.
@caesarxuchao can correct me if there was, since he wrote that code.

@k8s-bot
Copy link

k8s-bot commented Nov 2, 2015

GCE e2e test build/test passed for commit 326d333.

@nikhiljindal
Copy link
Contributor Author

@k8s-bot unit test this please

@nikhiljindal
Copy link
Contributor Author

Can I get an LGTM?
I want to cherrypick this

@caesarxuchao caesarxuchao added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 3, 2015
nikhiljindal added a commit that referenced this pull request Nov 3, 2015
Fixing missing swagger spec for apis/extensions
@nikhiljindal nikhiljindal merged commit 34cbe48 into kubernetes:master Nov 3, 2015
nikhiljindal added a commit that referenced this pull request Nov 4, 2015
…#16690-upstream-release-1.1

Automated cherry pick of #16690 upstream release 1.1
shyamjvs pushed a commit to shyamjvs/kubernetes that referenced this pull request Dec 1, 2016
…y-pick-of-#16690-upstream-release-1.1

Automated cherry pick of kubernetes#16690 upstream release 1.1
shouhong pushed a commit to shouhong/kubernetes that referenced this pull request Feb 14, 2017
…y-pick-of-#16690-upstream-release-1.1

Automated cherry pick of kubernetes#16690 upstream release 1.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Swagger spec: missing ApiDeclaration for /apis/extensions
7 participants