AWS Event Bridge: Allow passing DetailType #16828
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
WalkthroughThe changes introduce an optional Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Action
participant AWS_EventBridge
User->>Action: Provide eventData (and optionally detailType)
Action->>Action: Set DetailType = detailType if provided, else join keys of eventData
Action->>AWS_EventBridge: Send event with DetailType and eventData
AWS_EventBridge-->>Action: Acknowledge event
Action-->>User: Return result
Assessment against linked issues
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/aws/actions/eventbridge-send-event/eventbridge-send-event.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
components/aws/actions/eventbridge-send-event/eventbridge-send-event.mjs (2)
22-27: Well-documented detailType property with room for validation improvement.The property definition is comprehensive and correctly explains AWS EventBridge requirements. However, consider adding validation for the 128-character limit mentioned in the description.
Add validation to ensure the detailType doesn't exceed AWS limits:
detailType: { type: "string", label: "Detail Type", description: "Free-form string, with a maximum of 128 characters, used to decide what fields to expect in the event detail. Detail, DetailType, and Source are required for EventBridge to successfully send an event to an event bus. If you include event entries in a request that does not include each of those properties, EventBridge fails that entry. If you submit a request in which none of the entries have each of these properties, EventBridge fails the entire request.", optional: true, + max: 128, },
34-34: Implementation provides good backward compatibility but could be improved.The conditional logic correctly maintains backward compatibility while allowing user-specified DetailType. However, the fallback logic could create unexpectedly long or poorly formatted DetailType values.
Consider improving the fallback logic to handle edge cases:
- DetailType: this.detailType || Object.keys(this.eventData).join(" "), + DetailType: this.detailType || Object.keys(this.eventData).join(" ").substring(0, 128),This ensures the generated DetailType never exceeds AWS limits even when eventData has many keys.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
components/aws/actions/eventbridge-send-event/eventbridge-send-event.mjs(2 hunks)components/aws/package.json(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
🔇 Additional comments (3)
components/aws/package.json (2)
3-3: Appropriate version bump for new feature.The minor version increment is appropriate for adding the new
detailTypefunctionality.
32-32:❓ Verification inconclusive
Verify compatibility with major platform dependency update.
The dependency update from
^1.6.3to^3.0.3represents a major version jump. Ensure this update doesn't introduce breaking changes that affect existing functionality.
🏁 Script executed:
#!/bin/bash # Description: Check for any breaking changes or migration guides for @pipedream/platform v3.x # Expected: Documentation or changelog entries explaining breaking changes # Search for any migration guides or breaking change documentation rg -i "breaking|migration|changelog" --type mdLength of output: 2398
Review @pipedream/platform v3.x for breaking changes
I ran a search across our local docs and didn’t find any migration guide or changelog entries for the jump from v1.x to v3.x of
@pipedream/platform. Before merging, please:
- Consult the official changelog or release notes for
@pipedream/platformv3.x (e.g. on npm or the GitHub Releases page) to identify any breaking changes.- Verify that all existing workflows and API usages continue to function under v3.0.3.
- Update any code or tests if you encounter deprecated/removed methods.
components/aws/actions/eventbridge-send-event/eventbridge-send-event.mjs (1)
7-9: Metadata updates look good.The name capitalization fix, description simplification, and version bump are appropriate for this feature addition.
luancazarine
left a comment
luancazarine
left a comment
There was a problem hiding this comment.
Hi @michelle0927, LGTM! Ready for QA!
Resolves #16758
Summary by CodeRabbit
New Features
Improvements