make new-app error reports clearer #12978

gabemontero · 2017-02-15T20:07:02Z

@openshift/devex ptal

Some sample output:

gmontero ~/go/src/github.com/openshift/origin  (issue12892)$ oc new-app --strategy=docker https://192.30.253.113/openshift/ruby-hello-world.git --dry-run
error: Errors occurred while determining argument types:

https://192.30.253.113/openshift/ruby-hello-world.git as a Git repository URL:  fatal: unable to access 'https://192.30.253.113/openshift/ruby-hello-world.git/': Unable to communicate securely with peer: requested domain name does not match the server's certificate.

https://192.30.253.113/openshift/ruby-hello-world.git as a local directory pointing to a Git repository:  stat https://192.30.253.113/openshift/ruby-hello-world.git: no such file or directory

Errors occurred during resource creation:
error: --strategy is specified and none of the arguments provided could be classified as a source code location

gmontero ~/go/src/github.com/openshift/origin  (issue12892)$ oc new-app https://www.google.com/openshift/nodejs-e --dry-run
error: Errors occurred while determining argument types:

https://www.google.com/openshift/nodejs-e as a Git repository URL:  fatal: repository 'https://www.google.com/openshift/nodejs-e/' not found

https://www.google.com/openshift/nodejs-e as a local directory pointing to a Git repository:  stat https://www.google.com/openshift/nodejs-e: no such file or directory

Errors occurred during resource creation:
error: unable to load template file "https://www.google.com/openshift/nodejs-e": unable to read URL "https://www.google.com/openshift/nodejs-e", server reported 404 Not Found, status code=404
error: no match for "https://www.google.com/openshift/nodejs-e"

The 'oc new-app' command will match arguments to the following types:

  1. Images tagged into image streams in the current project or the 'openshift' project
     - if you don't specify a tag, we'll add ':latest'
  2. Images in the Docker Hub, on remote registries, or on the local Docker engine
  3. Templates in the current project or the 'openshift' project
  4. Git repository URLs or local paths that point to Git repositories

--allow-missing-images can be used to point to an image that does not exist yet.

See 'oc new-app -h' for examples.

gmontero ~/go/src/github.com/openshift/origin  (issue12892)$ oc new-app https://githb.com/openshift/nodejs-e --dry-run
error: Errors occurred while determining argument types:

https://githb.com/openshift/nodejs-e as a Git repository URL:  fatal: unable to access 'https://githb.com/openshift/nodejs-e/': Network file descriptor is not connected

https://githb.com/openshift/nodejs-e as a local directory pointing to a Git repository:  stat https://githb.com/openshift/nodejs-e: no such file or directory

Errors occurred during resource creation:
error: unable to load template file "https://githb.com/openshift/nodejs-e": Get https://githb.com/openshift/nodejs-e: dial tcp 158.69.145.48:443: i/o timeout
error: no match for "https://githb.com/openshift/nodejs-e"

The 'oc new-app' command will match arguments to the following types:

  1. Images tagged into image streams in the current project or the 'openshift' project
     - if you don't specify a tag, we'll add ':latest'
  2. Images in the Docker Hub, on remote registries, or on the local Docker engine
  3. Templates in the current project or the 'openshift' project
  4. Git repository URLs or local paths that point to Git repositories

--allow-missing-images can be used to point to an image that does not exist yet.

See 'oc new-app -h' for examples.

bparees · 2017-02-15T20:18:26Z

pkg/generate/app/cmd/newapp.go

+		remote, err := app.IsRemoteRepository(s)
+		c.ArgumentClassificationErrors["Remote access"] = err
+		local, err := app.IsDirectory(s)
+		c.ArgumentClassificationErrors["Local access"] = err


shouldn't this whole block be protected by the if !validated condition, to mimic the switch behavior?

also only set the classification errors is err!=nil

I was originally that way (whole block, etc .etc.), and then broke slightly from that norm in this sense:

The information as to whether the arg satisfied any of the tests seemed useful to me

note I don't update the c.Components etc. for any of the subsequent classifications on a match has been made ... in that sense I preserve the switch behavior

And on classificatnoi errors and err != nil, again, that was intentional. If a classification passed, I wanted it illustrated by the fact that the error field was nil. If I don't add a key / nil err pair, then unless you know the entire list of classifications, you don't know what has passed if we only list the actual errors.

I'm not sure how the additional information is useful since there's nothing the user can do about it (eg if we think it's an environment variable, it doesn't really matter if we also think it's a source repo, or are sure it's not a source repo because of error X, it's still going to get processed/consumed as an environment variable).

If anything it may just lead to more confusion since anytime new-app fails overall(for unrelated reasons), it's going to print out messages talking about the error it got when trying to treat an env variable argument as a source repo.

And on classificatnoi errors and err != nil, again, that was intentional. If a classification passed, I wanted it illustrated by the fact that the error field was nil. If I don't add a key / nil err pair, then unless you know the entire list of classifications, you don't know what has passed if we only list the actual errors.

yeah i sort of figured that was the intention, but for an end user it's a pretty ugly/confusing output. I might be convinced of keeping the nil value and doing better processing when printing the errors out to reformat the "nil" to something more informative to the user, but overall I think doing anything with it is tipping too far on the side of "what gabe+ben want to see to debug the problem" and not "what is useful to a user to see to know what they should do to fix it". For example "Component reference" means nothing at all to an end user.

After some internal ping-pong, I landed on not feeling strongly enough to dig in one side or the other :-) I'll switch back to the closer mimic of the switch behavior.

bparees · 2017-02-15T20:19:13Z

pkg/generate/app/cmd/newapp.go

+			validated = true
+		}
+
+		err = app.IsComponentReference(s)


same here, guard with if !validated

bparees · 2017-02-15T20:19:37Z

pkg/generate/app/cmd/newapp.go

+		}
+
+		err = app.IsComponentReference(s)
+		c.ArgumentClassificationErrors["Component reference processing"] = err


only set this if err!=nil?

bparees · 2017-02-15T20:20:29Z

pkg/generate/app/cmd/newapp.go

+		}
+
+		rc, err = app.IsPossibleTemplateFile(s)
+		c.ArgumentClassificationErrors["Template processing"] = err


same comments as above.

bparees · 2017-02-15T20:24:43Z

pkg/cmd/cli/cmd/newapp.go

@@ -612,7 +612,13 @@ func handleRunError(err error, baseName, commandName, commandPath string) error
 		transformError(err, baseName, commandName, commandPath, groups)
 	}
 	buf := &bytes.Buffer{}
+	fmt.Fprintf(buf, "Prior to API Object processing, here were the results from application argument classification:\n")


only print this if len(config.ArgumentClassificationErrors)>0
(in conjunction w/ not adding things to that map unless the value is non-nil, as mentioned below)

OK ... still debating the suggestions above, I agree with the len check.

bparees · 2017-02-15T20:26:00Z

pkg/cmd/cli/cmd/newapp.go

+	for key, value := range config.ArgumentClassificationErrors {
+		fmt.Fprintf(buf, fmt.Sprintf("\n%s:  %v\n", key, value))
+	}
+	fmt.Fprint(buf, "\n\nThen, the following errors were noted during API Object processing:\n\n")


s/noted/occurred/ but this test will also have to be reworked to account for the possibility that it is the first output, once you make the change i suggested above.

Will do rewording ... agreement on above suggestions still pending

smarterclayton · 2017-02-15T20:57:25Z

What the heck is "API Object processing" and why is it showing up in an error message to an end user?

gabemontero · 2017-02-15T21:07:11Z

:-) It was my attempt to categorize/summarize the fact that oc new-app will ultimately result in the creation of the objects specified in the template .... BuildConfigs, DeploymentConfigs, Services, Routes, Secrets.

Prior to the REST interactions with the master that result in creation, there is client side interpretation of the arguments that result in git invocations, file system checks, that could or could not fail for various reasons.

I tasked with exposing those failures for debug when oc new-app fails.

That opening phrase was an attempt to provided context.

But of course suggestions to change are appreciated (assuming you agree with exposing those errors if the command fails).

gabemontero · 2017-02-15T21:34:35Z

Perhaps replace the entire fist sentence with: "Attempts to classify the arguments resulted in these errors:"

The for the next sentence go with "The following errors occurred after argument classification:"

gabemontero · 2017-02-15T21:37:14Z

Updates to @bparees comments pushed.

Fixing "API Object processing" still pending ... waiting a bit to see what feedback arrives.

jim-minter · 2017-02-16T10:01:49Z

pkg/cmd/cli/cmd/newapp.go

@@ -612,7 +612,15 @@ func handleRunError(err error, baseName, commandName, commandPath string) error
 		transformError(err, baseName, commandName, commandPath, groups)
 	}
 	buf := &bytes.Buffer{}
+	if config.ArgumentClassificationErrors != nil && len(config.ArgumentClassificationErrors) > 0 {


config.ArgumentClassificationErrors != nil is unnecessary: len(nil) == 0 in golang.

jim-minter · 2017-02-16T10:02:39Z

pkg/cmd/cli/cmd/newbuild.go

@@ -271,7 +271,15 @@ func handleBuildError(err error, baseName, commandName, commandPath string) erro
 		transformBuildError(err, baseName, commandName, commandPath, groups)
 	}
 	buf := &bytes.Buffer{}
+	if config.ArgumentClassificationErrors != nil && len(config.ArgumentClassificationErrors) > 0 {


config.ArgumentClassificationErrors != nil is unnecessary: len(nil) == 0 in golang.

part of next push

jim-minter · 2017-02-16T10:03:36Z

pkg/generate/app/cmd/newapp.go

+			continue
+		}
+
+		// had to move off of elegant switch to capture errors for future display, but mimic


This seems more like a commit comment than a code comment ;)

It certainly is .... between your comment and the exchange I had with @bparees, I believe the explanation has been sufficiently proliferated :-)

deleted with next push

jim-minter · 2017-02-16T10:06:46Z

pkg/generate/app/cmd/newapp.go

 	for _, s := range args {
-		switch {


If you're ditching the switch statement, can you refactor into a separate function which then allows you to avoid all the nested ifs? i.e.:

if foo { // something return } if bar { // something else return }

rather than

if foo { // something } else { if bar { // something else } else { if baz ... } }

actually, the refactor into functions made me realize I could preserve the switch - thanks!... part of next push

jim-minter · 2017-02-16T10:13:01Z

Comments added. For what my view is worth, I feel pretty reluctant about this change on the basis that it feels like it's building more stuff on top of our shaky foundations and locking us more into them. I'm concerned this will make future code maintenance/cleanup harder, rather than easier.

If possible, I'd sooner see one of the following:

a more lightweight approach (if one exists) that satisfies the issue(s) raised, or
closing the issues won't fix, or
starting a document recording new-app shortcomings for a rewrite one day, or
implementing CLI arguments like --I'm-telling-you-this-is-a-remote-git-repo=https://www.github.com/openshift/nodejs-ex which would bypass our not-so-smart smarts for people to whom this kind of thing matters.

gabemontero · 2017-02-16T16:21:32Z

Updates pushed include:

replacement of the "API Object .." text
changes stemming from @jim-minter 's suggestions

On @jim-minter 's concerns outside of the code change comments:

not convinced this change makes future rework more difficult ... also, I think some of the rework stemming from @jim-minter 's code comments helps here
a more lightweight approach did occur to me ... specifically the usability / readability of kcmduti.MultipleErrors and its compressed format is insufficient for our needs here IMO (see here ... folks may have noticed I added a //TODO asking about it in these changes. It has been my experience though that moving off of k8s helper functions is sometimes a tough sell in PR review. Replacing that output with some additional textual context and perhaps multi-line listing of errors could be sufficient to alleviate the confusion noted by users in recent issues.
the tactical vs. strategic change argument wrt oc new-app has lost traction with me at least .... for all the talk of replacing the current oc new-app, we don't even have a card/epic on our trello board yet for actually doing that, and I still see after a quick search 3 cards on our backlog that call for enhancements on the current implementation ... more tactical changes seem to be the reality
I like the --I'm telling you how to classify the argument idea. That feels more like a card than an bug issue though. And I'm inclined to improve the readability of the oc new-app error output in some fashion irrespective of this addition, but at least acknowledge the rationale in solving this in that fashion.

My two cents. @bparees - based on the responses from various folks, where's your head wrt this?

bparees · 2017-02-16T19:11:30Z

I still want to see this done, the current experience is bad.

a more lightweight approach (if one exists) that satisfies the issue(s) raised, or

i'm not sure what is particularly heavyweight about this, curious if the changes resulting from your feedback make you feel any better about it.

closing the issues won't fix, or

definitely not a fan of this, the current error behavior is pretty baffling to users. (and this PR spawned from a user issue in which the user was so confused by the behavior that they thought new-app didn't support ip address based source repos)

starting a document recording new-app shortcomings for a rewrite one day, or

i'm not against also doing this, at this point i'm sure a lot of us have a mental wishlist of "if i was writing new-app from scratch...." and it would certainly not hurt to have that written down.

implementing CLI arguments like --I'm-telling-you-this-is-a-remote-git-repo=https://www.github.com/openshift/nodejs-ex which would bypass our not-so-smart smarts for people to whom this kind of thing matters.

that's what --code, --template, --image, etc are for. and it is true that in the case of the original issue the user hit, if they'd rerun the command w/ --code they'd have gotten a more useful error:

sample-app (sc_metadata)$ oc new-app --code=https://192.30.253.113/openshift/nodejs-ex --dry-run
error: shallow cloning repository "https://192.30.253.113/openshift/nodejs-ex" to "/tmp/gen058686116" failed: Cloning into '/tmp/gen058686116'...
fatal: unable to access 'https://192.30.253.113/openshift/nodejs-ex/': Unable to communicate securely with peer: requested domain name does not match the server's certificate.

but obviously this user did not understand the failure they'd hit sufficiently to realize new-app had mis-categorized the argument and that telling new-app what the arg type was might get them more information...so i'm not convinced that's a sufficient solution (as evidenced by it not having been sufficient for the user who hit this)

bparees · 2017-02-16T19:45:58Z

pkg/cmd/cli/cmd/newapp.go

@@ -612,7 +612,15 @@ func handleRunError(err error, baseName, commandName, commandPath string) error
 		transformError(err, baseName, commandName, commandPath, groups)
 	}
 	buf := &bytes.Buffer{}
+	if len(config.ArgumentClassificationErrors) > 0 {
+		fmt.Fprintf(buf, "Attempts to classify the arguments resulted in these errors:\n")


"Errors occurred while determining argument types:"

part of next push

bparees · 2017-02-16T19:46:19Z

pkg/cmd/cli/cmd/newapp.go

+		for key, value := range config.ArgumentClassificationErrors {
+			fmt.Fprintf(buf, fmt.Sprintf("\n%s:  %v\n", key, value))
+		}
+		fmt.Fprint(buf, "\nThe following errors occurred during application creation:\n")


"Errors occurred during resource creation:"

part of next push

bparees · 2017-02-16T19:51:03Z

pkg/cmd/cli/cmd/newapp.go

@@ -599,7 +599,7 @@ func retryBuildConfig(info *resource.Info, err error) runtime.Object {
 	return nil
 }

-func handleRunError(err error, baseName, commandName, commandPath string) error {
+func handleRunError(err error, baseName, commandName, commandPath string, config *newcmd.AppConfig) error {


the only difference between this and handleBuildError is which function is invoked for transformation...seems like an opportunity to collapse some code (pass the transform function in as an argument)

(and might make @jim-minter feel better about the weight of the changes :) )

part of next push

bparees · 2017-02-16T20:11:26Z

pkg/cmd/cli/cmd/newbuild.go

 	for _, group := range groups {
+		//TODO how coupled are we to kcmdutil.MultipleErrors ... format is compressed, non-optimal readability


i'm not seeing a current need to change it.

bparees · 2017-02-16T20:13:47Z

pkg/generate/app/cmd/newapp.go

+		c.Components = append(c.Components, s)
+		return true
+	}
+	c.ArgumentClassificationErrors["Component reference processing"] = err


i'd like to see another term used here that will have meaning to the user. I think this is essentially checking if the value is either:

an image

an imagestreamtag

a template that exists in the cluster(?)

if it weren't for 3 this would be easier to fix.

I'll work off of the terminology in the suggestion

bparees · 2017-02-16T20:15:18Z

pkg/generate/app/cmd/newapp.go

+	}
+
+	if derr != nil {
+		c.ArgumentClassificationErrors["Local access"] = derr


these two error keys don't make it clear the failure was related to trying to treat the argument as a source repo. (remote access to what? local access to what?)

I'll work off of the terminology in the suggestion

bparees · 2017-02-16T20:17:26Z

pkg/generate/app/cmd/newapp.go

+		case c.addEnvironmentArguments(s):
+		case c.addSourceArguments(s):
+		case c.addComponentArguments(s):
+		case c.addTemplateArguments(s):


if the user passes multiple arguments, this error accumulation and reporting logic breaks down. You need to accumulate the ArgumentClassificationErrors on a per argument basis.

yep I'll include the arg string in the key - good catch

bparees · 2017-02-16T20:18:11Z

test/cmd/newapp.sh

+os::cmd::expect_failure_and_text 'oc new-app https://githb.com/openshift/nodejs-e' 'Remote access:  '
+os::cmd::expect_failure_and_text 'oc new-build --strategy=docker https://192.30.253.113/openshift/ruby-hello-world.git' 'Remote access:  '
+os::cmd::expect_failure_and_text 'oc new-build https://www.google.com/openshift/nodejs-e' 'Remote access:  '
+os::cmd::expect_failure_and_text 'oc new-build https://githb.com/openshift/nodejs-e' 'Remote access:  '


add a test w/ multiple arguments that all fail.

part of next push

gabemontero · 2017-02-16T21:10:21Z

responses to latest @bparees comments pushed

jim-minter · 2017-02-17T10:08:10Z

pkg/cmd/cli/cmd/newapp.go

@@ -599,7 +599,7 @@ func retryBuildConfig(info *resource.Info, err error) runtime.Object {
 	return nil
 }

-func handleRunError(err error, baseName, commandName, commandPath string) error {
+func handleError(err error, baseName, commandName, commandPath string, config *newcmd.AppConfig, f func(err error, baseName, commandName, commandPath string, groups errorGroups)) error {


rename f to transformError?

sure more informative less terse ... part of next push

jim-minter · 2017-02-17T10:09:22Z

pkg/cmd/cli/cmd/newapp.go

+		for key, value := range config.ArgumentClassificationErrors {
+			fmt.Fprintf(buf, fmt.Sprintf("\n%s:  %v\n", key, value))
+		}
+		fmt.Fprint(buf, "\nErrors occurred during resource creation:\n")


Should this be split so printing the \n is inside the if clause and printing the rest of the text outside?

Yeah I debated putting the "Errors occurred during resource creation:" in even if there were no ArgumentClassificationErrors, simply to add some context and clarification around the terse kcmdutil.MultipleErrors output.

You've tipped me in that direction. Part of next push.

jim-minter · 2017-02-17T10:11:05Z

pkg/generate/app/cmd/newapp.go

+		c.Components = append(c.Components, s)
+		return true
+	}
+	c.ArgumentClassificationErrors[fmt.Sprintf("%s as a imagestream tag or docker image reference ", s)] = err


a->an in a imagestream tag

part of next push

jim-minter · 2017-02-17T10:12:52Z

pkg/generate/app/cmd/newapp.go

@@ -117,6 +117,8 @@ type AppConfig struct {

 	OSClient        client.Interface
 	OriginNamespace string
+
+	ArgumentClassificationErrors map[string]error


I wonder whether

ArgumentClassificationErrors []struct { Key string Value error }

may prove better, to keep ordering of messages?

hmm ... yeah, slightly more cumbersome to use, but the implementation does prescribe a precedence.

I'll prototype and see if it looks palatable.

and then I'll be quiet :)

:-)

change is palatable ... about to push

jim-minter · 2017-02-17T10:14:18Z

pkg/generate/app/cmd/newapp.go

+		return true
+	}
+	if err != nil {
+		c.ArgumentClassificationErrors[fmt.Sprintf("%s as a template in an accessible project", s)] = err


Is the text right given it says as possible template file above?

part of next push

jim-minter · 2017-02-17T10:17:20Z

pkg/generate/app/cmd/newapp.go

-		case app.IsPossibleTemplateFile(s):
-			glog.V(2).Infof("treating %s as possible template file\n", s)
-			c.Components = append(c.Components, s)
+		case c.addEnvironmentArguments(s):


wonder whether calling these functions tryToAddFoo might make this a little clearer

part of next push

gabemontero · 2017-02-20T15:12:22Z

@bparees pushed changes stemming from your last set of code comments and updated the sample outputs in the description

bparees · 2017-02-20T15:49:01Z

pkg/cmd/cli/cmd/newapp.go

+		}
+		fmt.Fprint(buf, "\n")
+	}
+	fmt.Fprint(buf, "Errors occurred during resource creation:\n")


this should also be conditional on there actually being at least one resource creation error.

The fact that we are in handleErrors at this point already ensures that there is at least one error. See the if err == nil return snippet at the top. I believe an additional check is redundant.

yup, i see now. thanks.

bparees · 2017-02-20T15:50:59Z

test/cmd/newapp.sh

+os::cmd::expect_failure_and_text 'oc new-app https://githb.com/openshift/nodejs-e' 'as a GIT repository URL:  '
+os::cmd::expect_failure_and_text 'oc new-build --strategy=docker https://192.30.253.113/openshift/ruby-hello-world.git' 'as a GIT repository URL:  '
+os::cmd::expect_failure_and_text 'oc new-build https://www.google.com/openshift/nodejs-e' 'as a GIT repository URL:  '
+os::cmd::expect_failure_and_text 'oc new-build https://githb.com/openshift/nodejs-e' 'as a GIT repository URL:  '


these all need to be fixed to Git

doh!! .. the one time I didn't run test-cmd :-)

script update pushed

bparees · 2017-02-20T16:40:57Z

lgtm but we're going to need a final signoff from @smarterclayton on the error messaging.

gabemontero · 2017-02-24T15:03:20Z

@smarterclayton bump on you take on updated error message examples in PR description - thanks

bparees · 2017-03-02T15:56:47Z

@smarterclayton bump for review of new error output.

bparees · 2017-03-06T02:07:35Z

@openshift/cli-review ptal.

juanvallejo · 2017-03-06T16:20:59Z

pkg/cmd/cli/cmd/newapp.go

+		}
+		fmt.Fprint(buf, "\n")
+	}
+	fmt.Fprint(buf, "Errors occurred during resource creation:\n")


nit: fmt.Fprintln could be used instead

switch to fmt.Fprintln pushed

juanvallejo · 2017-03-06T16:32:42Z

@fabianofranz this lgtm from a cli perspective

bparees · 2017-03-06T17:36:31Z

thanks @juanvallejo
[merge]

openshift-bot · 2017-03-06T17:37:29Z

[Test]ing while waiting on the merge queue

gabemontero · 2017-03-06T19:22:38Z

Looks like a new-app extended test is not happy:

oc new-app 
13:36:52   should fail with a --name longer than 58 characters
13:36:52   /tmp/openshift/tito/rpmbuild-originie34Li/BUILD/origin-1.5.0/_output/local/go/src/github.com/openshift/origin/test/extended/builds/new_app.go:51



• Failure [17.397 seconds]
13:36:52 [builds][Conformance] oc new-app
13:36:52 /tmp/openshift/tito/rpmbuild-originie34Li/BUILD/origin-1.5.0/_output/local/go/src/github.com/openshift/origin/test/extended/builds/new_app.go:52
13:36:52   should fail with a --name longer than 58 characters [It]
13:36:52   /tmp/openshift/tito/rpmbuild-originie34Li/BUILD/origin-1.5.0/_output/local/go/src/github.com/openshift/origin/test/extended/builds/new_app.go:51
13:36:52 
13:36:52   Expected
13:36:52       <string>: error: Errors occurred during resource creation:
13:36:52       error: invalid name: a2345678901234567890123456789012345678901234567890123456789. Must be an a lower case alphanumeric (a-z, and 0-9) string with a maximum length of 58 characters, where the first character is a letter (a-z), and the '-' character is allowed anywhere except the first or last character.
13:36:52   to have prefix
13:36:52       <string>: error: invalid name: 
13:36:52 
13:36:52   /tmp/openshift/tito/rpmbuild-originie34Li/BUILD/origin-1.5.0/_output/local/go/src/github.com/openshift/origin/test/extended/builds/new_app.go:50

Didn't realize this extended test existed. I believe it is getting confused by the new, additional error messages that were added.

I'll start working on an update to the extended test to account for the new messages.

gabemontero · 2017-03-06T19:26:41Z

Looks like something happened with pkg/generate/app/cmd/newapp_test.go after one of the rebases. Will need to address that too.

gabemontero · 2017-03-06T19:50:35Z

OK fixes to those two items pushed.

[test]

openshift-bot · 2017-03-06T19:53:29Z

Evaluated for origin test up to 5fa0f68

bparees · 2017-03-06T19:58:59Z

[merge]

openshift-bot · 2017-03-06T21:12:17Z

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_future/870/) (Base Commit: 5e64198)

openshift-bot · 2017-03-07T00:45:22Z

Evaluated for origin merge up to 5fa0f68

openshift-bot · 2017-03-07T02:13:19Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_future/870/) (Base Commit: da719a3) (Image: devenv-rhel7_6047)

bparees requested changes Feb 15, 2017

View reviewed changes

gabemontero force-pushed the issue12892 branch from a4ae8a9 to ff09cdc Compare February 15, 2017 21:35

jim-minter reviewed Feb 16, 2017

View reviewed changes

gabemontero force-pushed the issue12892 branch from ff09cdc to 023acd6 Compare February 16, 2017 15:54

bparees reviewed Feb 16, 2017

View reviewed changes

gabemontero force-pushed the issue12892 branch from 023acd6 to 75cabce Compare February 16, 2017 21:09

jim-minter reviewed Feb 17, 2017

View reviewed changes

bparees requested changes Feb 20, 2017

View reviewed changes

gabemontero force-pushed the issue12892 branch from 7d0cce7 to ab9137a Compare February 20, 2017 16:22

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 24, 2017

gabemontero force-pushed the issue12892 branch from ab9137a to c0eaf6f Compare February 24, 2017 16:08

openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 24, 2017

gabemontero force-pushed the issue12892 branch 2 times, most recently from 987dd12 to 0a11b3a Compare March 2, 2017 14:39

gabemontero force-pushed the issue12892 branch from 0a11b3a to 0522ec4 Compare March 6, 2017 13:53

juanvallejo reviewed Mar 6, 2017

View reviewed changes

gabemontero force-pushed the issue12892 branch from 0522ec4 to 5afe4de Compare March 6, 2017 17:31

make new-app error reports clearer

5fa0f68

gabemontero force-pushed the issue12892 branch from 5afe4de to 5fa0f68 Compare March 6, 2017 19:49

openshift-bot merged commit 4cd65ea into openshift:master Mar 7, 2017

gabemontero deleted the issue12892 branch March 7, 2017 14:08

		for _, group := range groups {
		//TODO how coupled are we to kcmdutil.MultipleErrors ... format is compressed, non-optimal readability

make new-app error reports clearer #12978

make new-app error reports clearer #12978

Uh oh!

Conversation

gabemontero commented Feb 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smarterclayton commented Feb 15, 2017

Uh oh!

gabemontero commented Feb 15, 2017

Uh oh!

gabemontero commented Feb 15, 2017

Uh oh!

gabemontero commented Feb 15, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jim-minter commented Feb 16, 2017

Uh oh!

gabemontero commented Feb 16, 2017

Uh oh!

bparees commented Feb 16, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

gabemontero commented Feb 15, 2017 •

edited

Loading