Eventbridge Event Processor #704
Conversation
BMorinDrifter
left a comment
BMorinDrifter
left a comment
There was a problem hiding this comment.
Some minor polish feedback, otherwise looks good to me.
|
|
||
| #[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] | ||
| #[serde(rename_all = "kebab-case")] | ||
| pub struct EventBridgeEvent { |
There was a problem hiding this comment.
Could this be an alias for EventBridgeEventObj<Option<String>> to avoid code duplication?
There was a problem hiding this comment.
We are having some problems creating this Alias due to the use of this setting in the "detail" field:
#[serde_as(as = "serde_with::json::JsonString")]
#[serde(bound(deserialize = "T: DeserializeOwned"))]
pub detail: T,As the input of this field will always be a String, we expect it to be a JsonString as an EventBridgeEventObj, but if you create an alias EventBridgeEventObj<Option> and its contents are a simple String, not a JsonString, the test will fail.
We're still working on solutions to avoid duplication, but so far we've had no success. Any suggestions?
There was a problem hiding this comment.
I'm ok with this duplication. We can change it later if we find a way.
|
Thanks a lot for opening this PR! I just merged some fixes for tests that were failing and were unrelated to your changes. Can you rebase from the main branch and resolve the conflicts? |
There was a problem hiding this comment.
Just one thing. Can you add this new feature here? https://github.com/awslabs/aws-lambda-rust-runtime/blob/main/Makefile#L66
Those tests are used to ensure that features still work in isolation. We used to have problems where some features had dependencies that were only enabled when you ran with all features enabled.
5 participants
Issue #, if available:
Description of changes:
New event created to support Eventbridge.
By submitting this pull request