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.

Sign up for GitHub

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

Jump to bottom

Change Metadata schema value from Dict to Any #60

Merged
merged 1 commit into from
May 5, 2021
Merged

Change Metadata schema value from Dict to Any #60

merged 1 commit into from
May 5, 2021

Conversation

donatoaz
Copy link
Contributor

According to AWS docs on the format of Metadata for a CloudFormation Template there is no hard requirement that values in the Metadata object be objects themselves. They can take any value.

This PR proposes to change Metadata schema to accept Any instead of a Dict such that the following metadata values are all acceptable:

"Metadata": {
    "Foo": "a_string_value",
    "Bar": 1
    "Fizz": ["an", "array"],
    "Buzz": { "an": "object" }
  },

@donatoaz
Copy link
Contributor Author

Also see #59

ocrawford555
ocrawford555 previously approved these changes May 4, 2021
View reviewed changes
Copy link
Contributor

@ocrawford555 ocrawford555 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the contribution!

@donatoaz
Change Metadata schema value from Dict to Any
a3c1912 
According to [AWS docs on the format of Metadata for a CloudFormation Template](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/metadata-section-structure.html) there is no hard requirement that values in the `Metadata` object be objects themselves. They can take any value.

This PR proposes to change `Metadata` schema to accept `Any` instead of a `Dict` such that the following metadata values are all acceptable:

```
"Metadata": {
    "Foo": "a_string_value",
    "Bar": 1
    "Fizz": ["an", "array"],
    "Buzz": { "an": "object" }
  },
```
jsoucheiron
jsoucheiron approved these changes May 4, 2021
View reviewed changes
Copy link
Member

@jsoucheiron jsoucheiron left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for contributing!
We'll add this in the next release

@donatoaz
Copy link
Contributor Author

donatoaz commented May 4, 2021

Hi! I've updated the pycfmodel/cloudformation_actions.py so tests would pass, and added two new test cases to tests/test_cf_model.py.

Let me know if you agree

@oscarbc96 oscarbc96 merged commit a3c4f19 into Skyscanner:master May 5, 2021
@oscarbc96 oscarbc96 linked an issue May 5, 2021 that may be closed by this pull request
Parse fails for Metadata containing string values #59
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsoucheiron jsoucheiron jsoucheiron approved these changes

@oscarbc96 oscarbc96 oscarbc96 approved these changes

@ocrawford555 ocrawford555 ocrawford555 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Parse fails for Metadata containing string values
4 participants
@donatoaz @jsoucheiron @oscarbc96 @ocrawford555