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

Deprecate asyncInit & account for other initialization types #659

Merged
merged 2 commits into from
Oct 25, 2023

Conversation

richarddd
Copy link

Issue #, if available:
Fixes: #507

Description of changes:
asyncInit() setting is now deprecated and always enabled unless users opt out by providing their own initializationWrapper. It's more sensible to provide a working default (that users can opt out of) than have a broken default depending on available resources and init times.

AsyncInitializationWrapper also falls back to InitializationWrapper if initialization mode is not "on-demand" to account for other execution modes (SnapStart and PC)

By submitting this pull request

  • I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • I confirm that I've made a best effort attempt to update all relevant documentation.

@richarddd richarddd changed the title Deprecate async init Deprecate async init & account for other initialization types Oct 20, 2023
@richarddd richarddd changed the title Deprecate async init & account for other initialization types Deprecate asyncInit & account for other initialization types Oct 20, 2023
Copy link
Collaborator

@deki deki 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 very much for your contribution @richarddd. We can include this for 2.0.0 so please change the comments accordingly.

Btw. as we are now making it the default, would you be interested to look into #276 as well?

@@ -165,7 +165,7 @@ public Builder responseTypeClass(Class<ResponseType> responseType) {
/**
* Uses an async initializer with the given start time to calculate the 10 seconds timeout.
*
* @deprecated As of release 1.5 this method is deprecated in favor of the parameters-less one {@link ServletLambdaContainerHandlerBuilder#asyncInit()}.
* @deprecated As of release 2.1 this method is deprecated. Initializer is always async if running in on-demand.
Copy link
Collaborator

Choose a reason for hiding this comment

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

should be 2.0.0 (currently we just have 2.0.0-M1 and 2.0.0-M2 milestone releases out there)

@@ -178,6 +178,7 @@ public Builder asyncInit(long actualStartTime) {
/**
* Uses a new {@link AsyncInitializationWrapper} with the no-parameter constructor that takes the actual JVM
* start time
* @deprecated As of release 2.1 this method is deprecated. Initializer is always async if running in on-demand.
Copy link
Collaborator

Choose a reason for hiding this comment

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

should be 2.0.0 (currently we just have 2.0.0-M1 and 2.0.0-M2 milestone releases out there)

@deki deki self-assigned this Oct 23, 2023
@deki deki added this to the Release 2.0 milestone Oct 23, 2023
@deki deki merged commit 854cace into aws:main Oct 25, 2023
4 checks passed
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.

AsyncInit only makes sense for regular lambda mode
3 participants