Skip to content
This repository has been archived by the owner on Oct 17, 2024. It is now read-only.

do not omit ReturnData in CloudWatch Data Metrics #254

Closed
wants to merge 1 commit into from

Conversation

taraspos
Copy link

If it is omitted CloudFormation template update with fail with the error: Exactly one element of the metrics list should return data, because all the omitted values assumed as True

From the AWS documentation:

ReturnData
This option indicates whether to return the timestamps and raw data values of this metric. If you are performing this call just to do math expressions and do not also need the raw data returned, you can specify False. If you omit this, the default of True is used.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

AWS documentation says 

*ReturnData*
This option indicates whether to return the timestamps and raw data values of this metric. If you are performing this call just to do math expressions and do not also need the raw data returned, you can specify False. *If you omit this, the default of True is used.*

Then, cloudformation template update with fail with the error: `Exactly one element of the metrics list should return data`, because  all the omitted values assumed as `True`
@taraspos
Copy link
Author

Ok, I see that code is autogenerated, but please, take a look when possible.

@manuel-trejo-rico
Copy link
Contributor

I have the same error because of ReturnData. The only workaround that worked for me is to use goformation lib as a vendor and modify Alarm_MetricDataQuery struct so it uses a *bool instead of a bool. This way, if not set it is omitted but if it is a pointer to false it is not omitted and the generated stack has a ReturnData = false field.

@taraspos
Copy link
Author

@goformation @PaulMaddox @grahamjenson could anyone take a look here, please? :)

@manuel-trejo-rico
Copy link
Contributor

Similar issue happens with UserPool Mutable field, if you set it to false it is omitted.

In general, I think that it would be a good idea to modify all the bool fields to be pointers to bool (*bool).

@manuel-trejo-rico
Copy link
Contributor

Similar issue happens with Cloudfront Default Cache Behaviour MinTTL, if you set '0' to forward all the headers the value is omitted. Changing from float64 to *float64 fix it

@grahamjenson
Copy link
Contributor

grahamjenson commented Mar 13, 2020

So the problem is that there are attributes where you want to send the 0 Value, but just removing the omitempty means you will ALWAYS send the 0 value.

I have run into this issue as well.

I think one way to solve this is to change the generated attributes to use pointer.

e.g. What it should look like is

ReturnData *bool `json:"ReturnData,omitempty"`

Once you change this, there will no doubt be other places you will have to change, given that after attributes can be nil.

What do you think @PaulMaddox?

@PaulMaddox
Copy link
Contributor

I'm currently writing an RFC for migrating this library to use pointers everywhere instead of values (and omitempty). This RFC should provide a better path moving forward for fixing this issue everywhere. Once the RFC is ready for review, I will share it for feedback in this repo/issue.

@rubenfonseca rubenfonseca added the v6 Issues that will be solved when v6 is released label Feb 25, 2022
@rubenfonseca
Copy link
Contributor

Hi! We believe this issue was fixed when we released v6 of the project. Please feel free to reopen if there's still a problem.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
on hold v6 Issues that will be solved when v6 is released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants