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

Documentation update: Adding Default Lookup to official docs #636

Merged
merged 3 commits into from
Aug 4, 2018

Conversation

GarisonLotus
Copy link
Contributor

This has been implemented since February.

@GarisonLotus
Copy link
Contributor Author

@phobologic - Just an FYI, no actual code updates in this PR.

default Lookup
--------------

The ``default`` lookup type will check if a value exists for the variable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't 100% true, right? It can also be used to give default variables to lookups.

${default ${output MyVpc::VpcId}::vpc-123456}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I... hadn't, really thought of that. It's an interesting idea. I didn't write the lookup module, so let me dig into it to check if that would work. (Or @troyready could chime in, since he wrote it)

@GarisonLotus
Copy link
Contributor Author

@phobologic - Troy and I just spoke about this idea, and we don't believe that this would work. Although we have not tested it, we think that stacker would throw an exception for the output lookup in your example, stoping the default fall back from completing.

I'm thinking it might be a good idea to make it more obvious that we mean "environment file" in the documentation, since that is how the feature actually was implemented.

EDIT: ok, I tested it out, yup - output lookup throws an exception, so I will update the docs to make it more obvious we are referring to environment files only.

Copy link
Member

@phobologic phobologic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I think it can work with other lookups, as long as they return a blank string. But this should be fine. Thanks!

@phobologic phobologic merged commit 342e04a into cloudtools:master Aug 4, 2018
phrohdoh pushed a commit to phrohdoh/stacker that referenced this pull request Dec 18, 2018
…ols#636)

* adding documentation for default lookup, missing when implmeneted in stacker since feb

* modifying lang to be only support environment file

* slight grammer fix
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.

2 participants