-
Notifications
You must be signed in to change notification settings - Fork 290
Revert "Adding send action for kinesis destination" #3408
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
base: main
Are you sure you want to change the base?
Conversation
This reverts commit dc96058.
New required fields detectedWarning Your PR adds new required fields to an existing destination. Adding new required settings/mappings for a destination already in production requires updating existing customer destination configuration. Ignore this warning if this PR is for a new destination with no active customers in production. The following required fields were added in this PR:
Add these new fields as optional instead and assume default values in |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3408 +/- ##
==========================================
+ Coverage 79.85% 80.26% +0.40%
==========================================
Files 1246 1502 +256
Lines 23758 29023 +5265
Branches 4808 6153 +1345
==========================================
+ Hits 18973 23294 +4321
- Misses 3952 4880 +928
- Partials 833 849 +16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| description: 'The name of the Kinesis stream to send records to', | ||
| type: 'string', | ||
| required: true, | ||
| disabledInputMethods: ['variable', 'function', 'freeform', 'enrichment'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you removing this?
| batch_keys?: string[] | ||
| /** | ||
| * The maximum number of payloads to include in a batch. | ||
| */ | ||
| max_batch_size: number |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are needed
| disabledInputMethods: ['variable', 'function', 'freeform', 'enrichment'] | ||
| }, | ||
| batch_keys: { | ||
| label: 'Batch Keys', | ||
| description: 'The keys to use for batching the events.', | ||
| type: 'string', | ||
| unsafe_hidden: true, | ||
| default: ['awsRegion', 'streamName', 'partitionKey'], | ||
| multiple: true | ||
| }, | ||
| max_batch_size: { | ||
| label: 'Max Batch Size', | ||
| description: 'The maximum number of payloads to include in a batch.', | ||
| type: 'number', | ||
| required: true, | ||
| minimum: 1, | ||
| maximum: 500, | ||
| default: 500 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of this stuff is needed.
|
AFAIK - the only thing we should be changing in this PR is to move the code from action-shared to lib. Please check with Mohammed. |
Reverts #3397
Reverting this PR because of SEV.