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

Note "Subject" is not optional for SNS messages #315

Closed
wants to merge 1 commit into from
Closed

Note "Subject" is not optional for SNS messages #315

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@

### 4.13.0

- Adds support for first-in-first-out (FIFO) SQS queues. https://github.com/mapbox/ecs-watchbot/pull/279
- Adds support for first-in-first-out (FIFO) SQS queues. https://github.com/mapbox/ecs-watchbot/pull/279.
- SNS message objects are required to have both a "Message" and a "Subject" property. Previously, "Subject" was optional.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you link to the issue ticket here? This wording sounds like it was an intentional change, but in fact it was a regression.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just so I understand, subjects used to be required, a regression was introduced at some point and it was fixed at version 4.13.0?

Copy link
Contributor

Choose a reason for hiding this comment

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

No. Subject used to be optional. Now they are required, i.e. the regression is that we accidentally introduced this requirement. More details at #304.

Copy link
Author

@sgillies sgillies Aug 22, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@vsmart the Travis errors were transient, I re-ran the builds and all is green.

Point taken about this being more of a regression. I don't usually edit my own change logs to retroactively note regressions introduced, only their fixes. Feel free to close this, though I would love to see some action. 3 different Mapbox projects stumbled over this.

Me and my team are fine about Subject being required, by the way. We went without them originally because we didn't know any better.


### 4.12.0
- Add `options.deadletterAlarm` (default=true) to disable the alarm resource for dead letter queue messages https://github.com/mapbox/ecs-watchbot/pull/288
Expand Down