Invalid target (-t) should not panic

[joanlopez]

A user input like target (-t) should not cause any panic, thus I think Grizzly should handle the error, in such case.

As an initial suggestion, I'm just skipping the target in case of error, cause if the glob cannot be compiled, there's no possible match. But we could want to give back some feedback to the user, either in form of a warning message, or interrupting the execution (in a controlled way, not panicking). What do you suggest?

Thanks!

[undef1nd]

Good catch, Joan. I guess returning an error to a user and stopping the execution should be good enough, cause if I am not mistaken Grizzly can't proceed without targets anyway.

[joanlopez]

Good catch, Joan. I guess returning an error to a user and stopping the execution should be good enough, cause if I am not mistaken Grizzly can't proceed without targets anyway.

That happens when checking resource matching with target, so I guess behaving as a "no match" and, perhaps, giving proper feedback would be enough (don't see the need to completely stop the execution). Indeed, what if only one of the targets is invalid while the rest remains correct? Why not using the others?

For instance, currently if you pass additional targets that make no sense, like: -t "AAA" -t "BBB" -t "Dashboard/*", they're just ignored, and only the meaningful ones are used. That's the reason why my initial commit just skips (continue) the target if it does not compile as glob. Wdyt?

[undef1nd]

Indeed, what if only one of the targets is invalid while the rest remains correct? Why not using the others?

Ah, sorry for the confusion: when writing the reply I was thinking of the case when for some reason none of the expressions gets compiled. So yes, what you're suggesting is fine. Just maybe worth adding some feedback for the user.

[malcolmholmes]

What happens when no targets are specified? Could it end up returning more resources rather than less?

[joanlopez]

What happens when no targets are specified?

The current behavior is: no targets means all targets

The code is:

func (r *registry) HandlerMatchesTarget(handler Handler, targets []string) bool {
	if len(targets) == 0 {
		return true
	}
        ...
}

So, all handlers does match.

Could it end up returning more resources rather than less?

I don't fully understand this question. In what scenario?

The changes added in this pull request, just make a "non-compilable" target (as defined by glob pattern spec) to behave the same as a target that is compilable but makes no sense / does not match, like: -t "StupidTarget", instead of crashing the application (panicking), which I guess is the desired behavior (at least the one that makes more sense to me).

Other than that, the follow up question is if we want to give some feedback to the user, but I guess that question could apply to both, non-compilable targets and "stupid" targets, so it can be shipped as a separate set of changes / pull request.