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

Support for S3 Lifecycle Rule property NoncurrentVersionTransitions #596

Merged
merged 6 commits into from
Nov 6, 2016

Conversation

SonOfBytes
Copy link
Contributor

Add support for S3 Lifecycle Rule move from singular NoncurrentVersionTransition to a set of NoncurrentVersionTransitions. Using same methodology as the deprecation of Transition.

Passes formatting, flake, and tests

Richard Adams added 5 commits October 20, 2016 11:23
…nTransition to a set of NoncurrentVersionTransitions. Using same methodology as the deprecation of Transition.
… removed erroneous code for delete 'object delete marker' which does not appear to be supported in CloudFormation at this point.
# aws moved from a single transition to a list of them
# and deprecated 'NoncurrentVersionTransition', so let's
# just move it to the new property and not annoy the user.
self.properties['NoncurrentVersionTransitions'] = [
Copy link
Member

Choose a reason for hiding this comment

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

Do you think it's worth adding a warning here to let the user know so they can transition their templates? At some point I'm assuming that Cloudformation will stop accepting the old format, so it might be nice to guide people along.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will always generate the new format even if you use the old standard in py. So the output will always be compliant. Given that I don't see a hard wall that someone will hit at some point down the road do we want to actively induce the change in their py code?

If we feel that a warning is the right way to go we can look at that.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think a warning may be a good idea, to encourage keeping troposphere as a thin layer above cloudformation.

Having a warning here also allows troposphere to stop supporting the old format and simplify the code base when increasing major version numbers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a preferred way to generate warnings in troposphere?

Copy link
Member

Choose a reason for hiding this comment

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

@phobologic phobologic merged commit 84556f8 into cloudtools:master Nov 6, 2016
@phobologic
Copy link
Member

Thanks @SonOfBytes !

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