fix(scale):dont call controller if there is no valid scale pattern

[kmala]

Output before:

keerthanmala$ deis scale web=-1
'web=-1' does not match the pattern 'type=num', ex: web=2
Scaling processes... but first, coffee!
done in 0s
=== woodsy-jokester Processes
--- cmd:
woodsy-jokester-v2-cmd-zbonf up (v2)

after:

deis scale web=-1 -a=woodsy-jokester
'web=-1' does not match the pattern 'type=num', ex: web=2

[deis-bot]

@Joshua-Anderson and @helgi are potential reviewers of this pull request based on my analysis of git blame information. Thanks @kmala!

[bacongobbler]

I think we should just make this return an error if there's a bad seed in the scale list. That would solve this problem quickly, cause ps:scale to return non-zero and we can test for it.

e.g.

return fmt.Errorf("'%s' does not match the pattern 'type=num', ex: web=2\n", target)

[kmala]

i am ok with returning and error but i think we are not returning an error because user can try to scale multiple proc types at a time and only few can be bad.

[bacongobbler]

yeah but what I mean is that if there is a single bad scale type we should error and say "hey, this process type is poorly formed, so we're not going to accept this request. Please try again."

Same thing applies for config:set as well. For example there were issues like #61 that was unintended, so there is precedence.

[bacongobbler]

also the current implementation will cause ps:scale to return 0 (meaning success) when they call deis ps:scale web=-1. That's not really what we would want to do in this scenario

[bacongobbler]

In the future we should probably clean this out so we don't have to have this ugly testing code to mock out the client settings, but this works for now. Thanks!

[Joshua-Anderson]

This will break on windows, cause $HOME is different. I wrote settings.SetHome to handle the cross platform differences transparently.

[Joshua-Anderson]

The way I've normally tested the stuff is by extracting the logic I want to test into its own function and then test that, which IMO is a bit cleaner. cmd.chooseKey is an example of this.

I also have some mock session code in settings_tests.go that could probably be made public and does all this work for you, if you want to use a session.

[bacongobbler]

that is indeed the better approach, but I am happy to merge this as-is and we can clean this up later.

[arschles]

can this become a struct that implements error?

[bacongobbler]

you mean like https://github.com/deis/controller-sdk-go/blob/master/errors.go? Those errors are for server-side, but that is a good suggestion.

[arschles]

not a global var, a global type since this has variable data in it:

type errInvalidScalePattern struct {
  target string
}

// Error implements the error interface
func (e errInvalidScalePattern) Error() string {
  return fmt.Sprintf("'%s' does not match the pattern 'type=num', ex: web=2\n", e.target)
}

[kmala]

this is something which needs to be done on the whole repo....may be i can create an issue and do it later?

[arschles]

@kmala an issue to do it later is fine, but why not start with this one now?

[kmala]

i had created an issue #129