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

Add Amazon MQ for RabbitMQ event structure #387

Merged
merged 5 commits into from Sep 17, 2021

Conversation

Infra-Red
Copy link
Contributor

@Infra-Red Infra-Red commented Aug 18, 2021

Description of changes:

Adds Amazon MQ for RabbitMQ event structures.

> go test -race  ./events
ok      github.com/aws/aws-lambda-go/events     0.605s

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

@Infra-Red Infra-Red changed the title Add Amazon MQ for RabbitMQ event structure Draft: Add Amazon MQ for RabbitMQ event structure Aug 19, 2021
@Infra-Red Infra-Red marked this pull request as draft August 19, 2021 07:23
Signed-off-by: Andrei Krasnitski <andrei.krasnitski@mendix.com>
@Infra-Red Infra-Red changed the title Draft: Add Amazon MQ for RabbitMQ event structure Add Amazon MQ for RabbitMQ event structure Aug 19, 2021
@Infra-Red Infra-Red marked this pull request as ready for review August 19, 2021 08:11
@Infra-Red
Copy link
Contributor Author

@bmoffatt Could you please review this change. Thanks!

Type *string `json:"type"`
UserId string `json:"userId"`
AppId *string `json:"appId"`
ClusterId *string `json:"clusterId"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Id -> ID everywhere

Really gotta get around to adding a linter rule for this, that's on me :)

type RabbitMQBasicProperties struct {
ContentType string `json:"contentType"`
ContentEncoding *string `json:"contentEncoding"`
Headers map[string]interface{} `json:"headers"` // Application or header exchange table
Copy link
Collaborator

Choose a reason for hiding this comment

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

note to self to double check the docs - sample data makes it seem like this almost could have been defined as

type RabbitMQBasicProperties struct {
  // ...
  Headers map[string]RabbitMQBasicPropertiesHeaders `json:"headers"`
  // ...
}
type RabbitMQBasicPropertiesHeaders struct {
  Bytes []byte `json:"bytes"`
}

and it'd be nice if possible if a user didn't have to drop down into having to reflect over an interface{} value

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see now that the sample data was copy+paste from the docs, so I guess there's not much that can be done here

@codecov-commenter
Copy link

codecov-commenter commented Sep 17, 2021

Codecov Report

Merging #387 (5603671) into main (4005b1d) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #387   +/-   ##
=======================================
  Coverage   71.63%   71.63%           
=======================================
  Files          19       19           
  Lines        1040     1040           
=======================================
  Hits          745      745           
  Misses        228      228           
  Partials       67       67           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4005b1d...5603671. Read the comment docs.

@bmoffatt bmoffatt merged commit d64324e into aws:main Sep 17, 2021
@Infra-Red Infra-Red deleted the rabbitmq-event-structure branch September 18, 2021 08:01
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.

None yet

3 participants