Skip to content

Conversation

@AntonEliatra
Copy link
Contributor

Description

adding the new config for aws lambda and defaults

Version

all

Checklist

  • By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and subject to the Developers Certificate of Origin.
    For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@github-actions
Copy link

Thank you for submitting your PR. The PR states are In progress (or Draft) -> Tech review -> Doc review -> Editorial review -> Merged.

Before you submit your PR for doc review, make sure the content is technically accurate. If you need help finding a tech reviewer, tag a maintainer.

When you're ready for doc review, tag the assignee of this PR. The doc reviewer may push edits to the PR directly or leave comments and editorial suggestions for you to address (let us know in a comment if you have a preference). The doc reviewer will arrange for an editorial review.

@AntonEliatra
Copy link
Contributor Author

@dlvenable can you please review this PR

Copy link
Member

@dlvenable dlvenable left a comment

Choose a reason for hiding this comment

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

Thank you @AntonEliatra for this improvement. I left some comments and maybe some for further discussion.

`invocation_type` | String | Required | Specifies the invocation type, either `request-response` or `event`. Default is `request-response`.
`aws.region` | String | Required | The AWS Region in which the Lambda function is located.
`aws.sts_role_arn` | String | Optional | The Amazon Resource Name (ARN) of the role to assume before invoking the Lambda function.
`function_name` | String | Required | The name of the AWS Lambda function to invoke. Default is `none`.
Copy link
Member

Choose a reason for hiding this comment

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

This is required. We should say that rather than "Default is none.

`aws.region` | String | Required | The AWS Region in which the Lambda function is located.
`aws.sts_role_arn` | String | Optional | The Amazon Resource Name (ARN) of the role to assume before invoking the Lambda function.
`function_name` | String | Required | The name of the AWS Lambda function to invoke. Default is `none`.
`aws.region` | String | Required | The AWS Region in which the Lambda function is located. Default is `none`.
Copy link
Member

Choose a reason for hiding this comment

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

The default value is not none. It will use the default from data-prepper-config.yaml if provided. Otherwise, I'm unsure what it is.

`aws.sts_role_arn` | String | Optional | The Amazon Resource Name (ARN) of the role to assume before invoking the Lambda function.
`function_name` | String | Required | The name of the AWS Lambda function to invoke. Default is `none`.
`aws.region` | String | Required | The AWS Region in which the Lambda function is located. Default is `none`.
`aws.sts_role_arn` | String | Optional | The Amazon Resource Name (ARN) of the role to assume before invoking the Lambda function. Default is `none`.
Copy link
Member

Choose a reason for hiding this comment

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

Similar comment as above. By default, this will use the configuration from data-prepper-config.yaml. If there are no configurations in either location, then it will use the default AWS credentials on the host.

`batch` | Object | Optional | The batch settings for the Lambda invocations. Default is `key_name = "events"`. Default threshold is `event_count=100`, `maximum_size="5mb"`, and `event_collect_timeout = 10s`.
`lambda_when` | String | Optional | A conditional expression that determines when to invoke the Lambda processor.
`batch` | Object | Optional | The batch settings for the Lambda invocations. Default is `key_name = "events"`, threshold `event_count=100`, `maximum_size="5mb"`, and `event_collect_timeout = 10s`.
`lambda_when` | String | Optional | A conditional expression that determines when to invoke the Lambda processor. Default is `none`.
Copy link
Member

Choose a reason for hiding this comment

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

Rather than "Default is none" I think we should say, "By default all events are processed."

`max_backoff` | Duration | Optional | The maximum backoff time for exponential backoff, in [ISO 8601 format](https://en.wikipedia.org/wiki/ISO_8601#Durations).
`max_concurrency` | Integer | Optional | The maximum concurrency defined on the client side.
`max_retries` | Integer | Optional | The maximum number of retries before failing.
`api_call_timeout` | Duration | Optional | The amount of time that the SDK maintains the API call before timing out, in [ISO 8601 format](https://en.wikipedia.org/wiki/ISO_8601#Durations). Default is `60s`.
Copy link
Member

Choose a reason for hiding this comment

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

This is actually a "Data Prepper duration." I'm not sure if we document this somewhere. We probably should have a data types section to define all these.

A Data Prepper duration can be expressed either as full ISO-8601 or as a simplified form in seconds or milliseconds: 60s or 60000ms.

`max_backoff` | Duration | Optional | The maximum backoff time for exponential backoff, in [ISO 8601 format](https://en.wikipedia.org/wiki/ISO_8601#Durations).
`max_concurrency` | Integer | Optional | The maximum concurrency defined on the client side.
`max_retries` | Integer | Optional | The maximum number of retries before failing.
`api_call_timeout` | Duration | Optional | The amount of time that the SDK maintains the API call before timing out, in [ISO 8601 format](https://en.wikipedia.org/wiki/ISO_8601#Durations). Default is `60s`.
Copy link
Member

Choose a reason for hiding this comment

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

It may be better to be more specific. The api_call_timeout is actually the total time that the SDK will attempt which includes all retries.

Signed-off-by: Anton Rubin <[email protected]>
@AntonEliatra
Copy link
Contributor Author

@dlvenable thats updated now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 3.3 Tech review PR: Tech review in progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants