Try to fixup #2117 by mattmoor · Pull Request #2586 · knative/serving

mattmoor · 2018-11-30T02:53:37Z

This is me trying to fixup and finish #2117

googlebot · 2018-11-30T02:53:40Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

knative-prow-robot · 2018-11-30T02:53:44Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mattmoor

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [mattmoor]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

knative-prow-robot

@mattmoor: 4 warnings.

Details

In response to this:

This is me trying to fixup and finish #2117

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

knative-prow-robot · 2018-11-30T02:53:48Z

+							}
+
+							if !equality.Semantic.DeepEqual(c.Resources, want) {
+								return false, fmt.Errorf("Invalid resource configuration for pod %v. Want: %+v, got: %+v.", pod.Name, want, c.Resources)


Golint errors: error strings should not be capitalized or end with punctuation or a newline. More info.

knative-prow-robot · 2018-11-30T02:53:48Z

+
+							if !equality.Semantic.DeepEqual(c.Resources, want) {
+								return false, fmt.Errorf("Invalid resource configuration for pod %v. Want: %+v, got: %+v.", pod.Name, want, c.Resources)
+							} else {


Golint indent: if block ends with a return statement, so drop this else and outdent its block. More info.

knative-prow-robot · 2018-11-30T02:53:48Z

+		}
+		want := "Moo!"
+		if want != strings.TrimSpace(string(response.Body)) {
+			return fmt.Errorf("The response '%s' is not equal to expected response '%s'.", string(response.Body), want)


Golint errors: error strings should not be capitalized or end with punctuation or a newline. More info.

knative-prow-robot · 2018-11-30T02:53:48Z

+			fmt.Fprintf(w, "cannot convert %s to int", memoryInMB)
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		} else {


Golint indent: if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary). More info.

mattmoor · 2018-11-30T03:43:47Z

/test pull-knative-serving-unit-tests

mattmoor · 2018-11-30T04:09:55Z

Let's see if it sticks...

/test pull-knative-serving-unit-tests

mattmoor · 2018-11-30T04:16:33Z

Wrong one to retry 😭

/test pull-knative-serving-integration-tests

mattmoor · 2018-11-30T04:41:10Z

$ go test -tags=e2e -count=150 -timeout=30m ./test/e2e -run=TestCustomResourcesLimits
ok      github.com/knative/serving/test/e2e     962.432s

Not bad :)

mattmoor · 2018-11-30T04:43:12Z

@bbrowning Since you were familiar with this, would you mind reviewing my additional changes?

mattmoor · 2018-11-30T04:51:25Z

/lint

knative-prow-robot

@mattmoor: 2 warnings.

Details

In response to this:

/lint

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

knative-prow-robot · 2018-11-30T04:51:35Z

+							}
+
+							if !equality.Semantic.DeepEqual(c.Resources, want) {
+								return false, fmt.Errorf("invalid resource configuration for pod %v. Want: %+v, got: %+v.", pod.Name, want, c.Resources)


Golint errors: error strings should not be capitalized or end with punctuation or a newline. More info.

knative-prow-robot · 2018-11-30T04:51:35Z

+			return err
+		}
+		if want != strings.TrimSpace(string(response.Body)) {
+			return fmt.Errorf("the response '%s' is not equal to expected response '%s'.", string(response.Body), want)


Golint errors: error strings should not be capitalized or end with punctuation or a newline. More info.

mattmoor · 2018-11-30T04:56:49Z

/lint

knative-prow-robot

@mattmoor: 0 warnings.

Details

In response to this:

/lint

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[knative/serving#knative#2099]

mattmoor · 2018-11-30T06:54:13Z

One more time for good measure:

/test pull-knative-serving-integration-tests

bbrowning · 2018-11-30T10:52:04Z

Ahh nice - so it was easy to fix this test by just filling out the entire array when bloating memory instead of just first/last elements.

/lgtm

mattmoor · 2018-11-30T14:06:06Z

@bbrowning yeah, I had to add a WaitForEndpointState as well because even with that the 100M request will sometimes fail.

mattmoor · 2018-11-30T14:36:19Z

I'm going to override branch protections to merge this. @jszroberto signed the CLA and provided the base PR, this just addresses test/lint issues.

knative-prow-robot requested review from dprotaso and dushyanthsc November 30, 2018 02:53

knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 30, 2018

knative-prow-robot reviewed Nov 30, 2018

View reviewed changes

mattmoor force-pushed the allow-resources branch from 6abeb8b to e49bfba Compare November 30, 2018 03:37

mattmoor assigned bbrowning Nov 30, 2018

knative-prow-robot reviewed Nov 30, 2018

View reviewed changes

mattmoor force-pushed the allow-resources branch from e20b409 to 04d8527 Compare November 30, 2018 04:56

knative-prow-robot reviewed Nov 30, 2018

View reviewed changes

MissingRoberto and others added 2 commits November 30, 2018 05:52

Allow resources in Revision

f954888

[knative/serving#knative#2099]

Initialize all of the allocated memory.

d7a0937

mattmoor force-pushed the allow-resources branch from 04d8527 to 46e7814 Compare November 30, 2018 05:55

Fix golint errors.

a419b00

mattmoor force-pushed the allow-resources branch from 46e7814 to a419b00 Compare November 30, 2018 06:01

mattmoor assigned dgerd Nov 30, 2018

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 30, 2018

mattmoor merged commit 7b07d51 into knative:master Nov 30, 2018

mattmoor deleted the allow-resources branch November 30, 2018 14:38

Conversation

mattmoor commented Nov 30, 2018

Uh oh!

googlebot commented Nov 30, 2018

Uh oh!

knative-prow-robot commented Nov 30, 2018

Uh oh!

knative-prow-robot left a comment

Choose a reason for hiding this comment

Uh oh!

knative-prow-robot Nov 30, 2018

Choose a reason for hiding this comment

Uh oh!

knative-prow-robot Nov 30, 2018

Choose a reason for hiding this comment

Uh oh!

knative-prow-robot Nov 30, 2018

Choose a reason for hiding this comment

Uh oh!

knative-prow-robot Nov 30, 2018

Choose a reason for hiding this comment

Uh oh!

mattmoor commented Nov 30, 2018

Uh oh!

mattmoor commented Nov 30, 2018

Uh oh!

mattmoor commented Nov 30, 2018

Uh oh!

mattmoor commented Nov 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattmoor commented Nov 30, 2018

Uh oh!

mattmoor commented Nov 30, 2018

Uh oh!

knative-prow-robot left a comment

Choose a reason for hiding this comment

Uh oh!

knative-prow-robot Nov 30, 2018

Choose a reason for hiding this comment

Uh oh!

knative-prow-robot Nov 30, 2018

Choose a reason for hiding this comment

Uh oh!

mattmoor commented Nov 30, 2018

Uh oh!

knative-prow-robot left a comment

Choose a reason for hiding this comment

Uh oh!

mattmoor commented Nov 30, 2018

Uh oh!

bbrowning commented Nov 30, 2018

Uh oh!

mattmoor commented Nov 30, 2018

Uh oh!

mattmoor commented Nov 30, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mattmoor commented Nov 30, 2018 •

edited

Loading