Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Improve optional parameter support #251

Closed
wants to merge 2 commits into from

Conversation

jthomas
Copy link
Member

@jthomas jthomas commented Jan 15, 2018

I've hit a bug where the script doesn't allow optional parameters, even though the script supports this.

If the CATALOG_AUTH_KEY parameter is missing, the script will extract the value from:

-CATALOG_AUTH_KEY=${1:-"$OPENWHISK_HOME/ansible/files/auth.whisk.system"}

However, if I miss this parameter on the command-line, it assumes there are only two parameters, not three and fails. If I pass an empty string to force a null parameter, this doesn't pass through to the source "$SCRIPTDIR/validateParameter.sh" $1 $2 $3 correctly.

I made an attempt to fix this by using environment variables for all parameters as they are all optional.

Moves script configuration from parameters to environment variables.
This makes it easier to handle optional values which currently fails.
jthomas added a commit to jthomas/incubator-openwhisk-devtools that referenced this pull request Jan 19, 2018
ddragosd pushed a commit to apache/openwhisk-devtools that referenced this pull request Jan 25, 2018
* Add Makefile targets for adding catalog

Fixes #6

* Revert catalog install to using script arguments.

Removes dependency on apache/openwhisk-catalog#251
@rabbah
Copy link
Member

rabbah commented Feb 11, 2019

Will we need this if we use wskdeploy #268?

@dgrove-oss
Copy link
Member

we removed installCatalog.sh; closing this PR as out-of-date.

@dgrove-oss dgrove-oss closed this Sep 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants