-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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
remove go-restful from namer for rest handling #43767
Conversation
|
@k8s-bot non-cri e2e test this |
@@ -746,7 +757,7 @@ func TestNotFound(t *testing.T) { | |||
"groupless root DELETE with extra segment": {"DELETE", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/simpleroots/bar/baz", http.StatusNotFound}, | |||
"groupless root PUT without extra segment": {"PUT", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/simpleroots", http.StatusMethodNotAllowed}, | |||
"groupless root PUT with extra segment": {"PUT", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/simpleroots/bar/baz", http.StatusNotFound}, | |||
"groupless root watch missing storage": {"GET", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/watch/", http.StatusNotFound}, | |||
"groupless root watch missing storage": {"GET", "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/watch/", http.StatusInternalServerError}, |
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 these change?
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 these change?
This used to test the cases without the "normal" handler chain. Once you add a requestInfoResolver into the chain, it fails on these URLs because they aren't well formed. The same thing would happen in our real chain.
@@ -1169,6 +1180,7 @@ func TestNonEmptyList(t *testing.T) { | |||
if err != nil { | |||
t.Fatalf("unexpected error: %v", err) | |||
} | |||
t.Log(body) |
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.
debug output
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.
debug output
Super useful when the test goes poorly.
objNamespace, objName, err := namer.ObjectName(obj) | ||
if err != nil { | ||
return errors.NewBadRequest(fmt.Sprintf( | ||
"the name of the object (%s based on URL) was undeterminable %v", name, err)) |
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.
undeterminable: %v
c1f0f69
to
da27957
Compare
Nit updated. Other comments? |
/lgtm |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: deads2k, sttts
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
vendor/build again. marking |
@k8s-bot test this |
bot can be up to 10 minutes behind, but I'm betting this is more than 10 minutes... |
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
Automatic merge from submit-queue |
Automatic merge from submit-queue (batch tested with PRs 43273, 44287, 44281) Remove ObjectMetaFor Builds on #43767 The second commit removes `ObjectMetaFor`. This was debt we left around after we created the interfaces. Fixing this makes it possible to start running `Unstructured` through generic storage. @kubernetes/sig-api-machinery-pr-reviews @smarterclayton @lavalamp
Automatic merge from submit-queue (batch tested with PRs 43273, 44287, 44281) Remove ObjectMetaFor Builds on kubernetes/kubernetes#43767 The second commit removes `ObjectMetaFor`. This was debt we left around after we created the interfaces. Fixing this makes it possible to start running `Unstructured` through generic storage. @kubernetes/sig-api-machinery-pr-reviews @smarterclayton @lavalamp Kubernetes-commit: d4eaf0b6801900d2b2f80036e267a1d3d86fd3a8
Our RESTHandler code is currently tightly coupled to go-restful, but there's no reason for this coupling. It makes integrations that want API handling (decode, sanity check, admission, verb handling), but don't need the REST installer flow impractical. I know of two layers now: metrics and TPR.
This starts the process of unwinding by switching the
ScopeNamer
(used for request identification and selflinks) to use the standard http library along with theRequestInfo
we place in the context for authorization and any other interested layer.@kubernetes/sig-api-machinery-misc @smarterclayton @ncdc @sttts