8000 Fix an issue introduced in #594 by phobologic · Pull Request #596 · cloudtools/stacker · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix an issue introduced in #594 #596

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

Merged
merged 1 commit into from
May 14, 2018
Merged

Fix an issue introduced in #594 #596

merged 1 commit into from
May 14, 2018

Conversation

phobologic
Copy link
Member

594 assumed that dump would use False as it's default, but since
--dump is a str type, it instead uses None when it is not
specified on the CLI.

The truth is that this is more pythonic anyway, and has the same effect.
I can't imagine when this could cause un-expected behaivior.

594 assumed that `dump` would use `False` as it's default, but since
`--dump` is a `str` type, it instead uses `None` when it is not
specified on the CLI.

The truth is that this is more pythonic anyway, and has the same effect.
I can't imagine when this could cause un-expected behaivior.
@phobologic
Copy link
Member Author

@zaro0508 can you verify this is ok for what you were thinking?

Note: This was hard to detect. Integration tests in my environment worked. Why? Because I don't delete my s3 bucket between runs, so even though ensure_cfn_bucket_exists was returning True, the bucket already existed.

@phobologic phobologic requested a review from ejholmes May 13, 2018 03:46
@phobologic phobologic merged commit d96c2be into master May 14, 2018
@phobologic phobologic deleted the fix_ensure_cfn_bucket branch May 14, 2018 23:29
@@ -83,7 +83,8 @@ def should_ensure_cfn_bucket(outline, dump):
bool: If access to CF bucket is needed, return True.

"""
return outline is False and dump is False
print "OUTLINE: %s, DUMP: %s" % (outline, dump)
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this intended to be left in? Was totally confused why my logs started showing this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Almost definitely not. I can pull it.

phrohdoh pushed a commit to phrohdoh/stacker that referenced this pull request Dec 18, 2018
594 assumed that `dump` would use `False` as it's default, but since
`--dump` is a `str` type, it instead uses `None` when it is not
specified on the CLI.

The truth is that this is more pythonic anyway, and has the same effect.
I can't imagine when this could cause un-expected behaivior.
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.

3 participants
0