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

Use a dict instead of the Tags object for the Tags prop for Dax #1046

Merged
merged 2 commits into from
Jan 22, 2019

Conversation

jswoods
Copy link
Contributor

@jswoods jswoods commented May 16, 2018

Cloudformation expects a dict rather than a list of key value pairs which the Tags object provides. See https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-dax-cluster.html.

This fixes #1045

@markpeek
Copy link
Member

@cmmeyer would you happen to know why dax appears to be using a totally different Tags format? This sounds like more of a CloudFormation defect to me.

@jswoods
Copy link
Contributor Author

jswoods commented May 16, 2018

@markpeek there appears to be two ways to handle tags in cloudformation. If you look at https://github.com/cloudtools/troposphere/blob/master/troposphere/batch.py#L19 for example, it uses a dict. I'm sure there are others as well.

@cmmeyer
Copy link

cmmeyer commented May 16, 2018

This may be an artifact of the new self-service model for CloudFormation support. When in doubt, the resource specification is your best friend for confirming/validating types:

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/cfn-resource-specification.html

    "AWS::DAX::Cluster": {
...
        "Tags": {
          "Required": false,
          "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-dax-cluster.html#cfn-dax-cluster-tags",
          "PrimitiveType": "Json",
          "UpdateType": "Mutable"
        }

Doing some quick sampling, there's definitely inconsistency here -- most are "List", a couple are "JSON" and (interestingly) some are "Tag".

Lemme check in with the team and see if I can understand why some are difference, might just be something we can call out in on-boarding.

@jswoods
Copy link
Contributor Author

jswoods commented May 24, 2018

@cmmeyer - for my own clarity, are you saying this fix is valid or is not valid? If not, what would be the proposed solution to #1045?

@jswoods
Copy link
Contributor Author

jswoods commented Aug 2, 2018

Any update on this?

@cmmeyer
Copy link

cmmeyer commented Aug 6, 2018

If this fixes tags for DAX then it should be the right call. I don't think you're going to see anything in the API/CloudFormation to normalize tags anytime soon.

@adamdavis40208
Copy link

Can this be merged? Tagging DAX resources would be a nice-to-have

@r631269
Copy link

r631269 commented Sep 25, 2018

+1, could really use this soon

@adamdavis40208
Copy link

@markpeek This PR has been around for ages, and it'd be nice to have it pulled in. Is there a better spot to discuss a merging this? (google group/IRC/etc)

Thanks!

@adamdavis40208
Copy link

Hello 2019. Any updates on this?

@markpeek markpeek merged commit dcb7c27 into cloudtools:master Jan 22, 2019
@markpeek
Copy link
Member

Thanks. Sorry for the delay and thanks for the constant reminders. I was hoping the CloudFormation team would "fix" this incompatibility but it is better to move forward with what works.

@adamdavis40208
Copy link

Awesome! Now of course amazon will fix it, which will cause another bug :)

That's how the universe works

@cmmeyer
Copy link

cmmeyer commented Jan 22, 2019

Sorry, folks! The distributed nature of the services means it's almost impossible to normalize this sort of thing across the platform. I'll keep this bookmarked and try to give you a heads up if I hear anything.

davemasino pushed a commit to davemasino/troposphere that referenced this pull request Oct 17, 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.

Cloudformation error on tags type validation when provisioning dax resource
5 participants