Skip to content
This repository has been archived by the owner on May 16, 2023. It is now read-only.

Bugfix/stringlistvalue #160

Merged
merged 2 commits into from
Apr 25, 2023
Merged

Conversation

setoelkahfi
Copy link
Contributor

@setoelkahfi setoelkahfi commented Apr 25, 2023

Probably related: #81

The string_list_values and binary_list_values are not required in the MessageAttributes object of the SQS payload. Doc.

We encountered a bug where the lambda throws an error with a message attributes like this:


{
    "action": {
        "DataType": "String",
        "StringValue": "CREATE"
    }
}

CHANGES

  • Add #[serde(default)] for non-required vectors.

pub string_list_values: Vec<String>,
#[serde(default)]
Copy link

@lbeschastny lbeschastny Apr 25, 2023

Choose a reason for hiding this comment

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

While serde(default) fixes the issue here, it also makes the absence of a value here undistinguishable from an empty array.
So, perhaps Option<Vec<String>> would be a better choice here.

But we should also keep in mind that those two fields are not actually used for anything yet and just reserved for future use (see AWS docs here).
So, it’s hard to tell now whether this limitation may cause any problems in the future or not.

Copy link
Owner

Choose a reason for hiding this comment

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

I'm ok with these changes. I don't think the difference between "absence" vs "empty" will be really meaningful in thsi case.

@calavera calavera merged commit 89b8c01 into calavera:main Apr 25, 2023
@setoelkahfi setoelkahfi deleted the bugfix/stringlistvalue branch April 25, 2023 15:08
@setoelkahfi
Copy link
Contributor Author

@calavera can we make a release patch for this?

@calavera
Copy link
Owner

@setoelkahfi I just released 0.8.4 with this patch.

@setoelkahfi
Copy link
Contributor Author

@calavera thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants