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

Implement support for function-timeout configuration #1852

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

unholyranger-work
Copy link

@unholyranger-work unholyranger-work commented Oct 29, 2024

Issue #665

Description of changes:
The templates deploy a function-timeout field which should correlate to the number of seconds until the lambda function times out and exits. This is a requirement for functions that rely on the ILambdaContext.RemainingTime as a CancellationTokenSoucre. The remaining time is always set to TimeSpan.Zero currently making it harder to test, or requiring to break and manually set the value.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@UnholyRanger
Copy link

Hey @96malhar and @philasmar, could you please review? Thanks

@normj
Copy link
Member

normj commented Nov 17, 2024

This is an interesting change for the test tool and wonder if would disrupt user's debugging in other ways. For example I can see users having function-timeout set to something small for production purposes like 10 seconds. But if we take this change as is and the value is used as the CancellationTokenSource then users would only have 10 seconds to debug.

Is the currently struggle that the value is 0 and that isn't an acceptable value for CancellationTokenSource. Wondering if instead of taking the function-timeout from the config file which is meant for deployment should the test tool set the RemainingTime to some high value so that the same code can be used for setting up the CancellationTokenSource but we would not interfere with debugging. Otherwise I think we have to come up some sort of opt-in mechanism to set the RemainingTime instead of always taking deployment config files function-timeout value.

@unholyranger-work
Copy link
Author

unholyranger-work commented Nov 18, 2024

Thanks for the reply @normj!
The current struggle is that since we're not loading in the ILambdaContext.RemainingTime which is supposed to be equivalent to the lambda's configured Timeout, the value is always 0 seconds. For lambdas where we want to cancel before AWS terminates the run, we need to know how long we have to operate. In my example, we allow the full 15-minute runtime but give 2 minutes to gracefully shut down. So any runs that go long, can stop processing, upload logs, etc, and exit gracefully without being terminated.
In my view, passing the config value into their appropriate context properties brings the test tool closer to how Lambda functions work. This is also why I default to 15 minutes when the value is not set. For anyone using the remaining time as a cancellation token, when checking cancellationToken.IsCancellationRequested it is always true. In my case, I have a loop at the start that exits right away. For anyone with short time frames, anything is better than 0. I have to hit a breakpoint and manually update the value before continuing.

@UnholyRanger
Copy link

Hey @normj @96malhar @philasmar,
I wanted to follow up to see if there is any additional feedback on this issue?

Thanks

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.

3 participants