make INVOKER_NAME optional #2902
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tysonnorris
requested changes
Oct 25, 2017
| @@ -60,7 +60,7 @@ object Invoker { | |||
| invokerContainerDns -> "", | |||
| invokerContainerNetwork -> null, | |||
| invokerUseRunc -> "true") ++ | |||
| Map(invokerName -> null) | |||
| Map(invokerName -> "") | |||
There was a problem hiding this comment.
invokerName should be validated as non-empty below, same as redisHost (it will only be validated when the id arg is not present)
There was a problem hiding this comment.
good point @tysonnorris.
done
tysonnorris
approved these changes
Oct 25, 2017
|
LGTM |
|
@rabbah , @markusthoemmes , @dgrove-oss I'd like to merge this to fix the build on |
rabbah
reviewed
Oct 26, 2017
rabbah
approved these changes
Oct 26, 2017
There was a problem hiding this comment.
Ok clarified on Slack. @tysonnorris had a useful comment perhaps worth adding here “if id arg is provided, use it, and do NOT require INVOKER_NAME or REDIS_HOST configs; if id arg is NOT provided, then DO require INVOKER_NAME AND REDIS_HOST configs”
houshengbo
pushed a commit
to houshengbo/openwhisk
that referenced
this pull request
Nov 13, 2017
* make INVOKER_NAME optional * abort on empty invoker name when redis host is specified
houshengbo
pushed a commit
to houshengbo/openwhisk
that referenced
this pull request
Nov 13, 2017
* make INVOKER_NAME optional * abort on empty invoker name when redis host is specified
houshengbo
pushed a commit
to houshengbo/openwhisk
that referenced
this pull request
Nov 14, 2017
* make INVOKER_NAME optional * abort on empty invoker name when redis host is specified
houshengbo
pushed a commit
to houshengbo/openwhisk
that referenced
this pull request
Nov 15, 2017
* make INVOKER_NAME optional * abort on empty invoker name when redis host is specified
houshengbo
pushed a commit
to houshengbo/openwhisk
that referenced
this pull request
Nov 17, 2017
* make INVOKER_NAME optional * abort on empty invoker name when redis host is specified
BillZong
pushed a commit
to BillZong/openwhisk
that referenced
this pull request
Nov 18, 2019
* make INVOKER_NAME optional * abort on empty invoker name when redis host is specified
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This slipped in with the Redis changes and I don't think it's needed b/c the invoker already starts with an
id.if
id argis provided, use it, and do NOT require INVOKER_NAME or REDIS_HOST configs; ifid argis NOT provided, then DO require INVOKER_NAME AND REDIS_HOST configsRelates to: #2872 and apache/openwhisk-devtools#62