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

chore: add Lambda configuration with values ported from nodejs #251

Merged
merged 3 commits into from
Nov 22, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions src/Config/Configurations/Lambda.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php
declare(strict_types=1);

namespace Momento\Config\Configurations;

use Momento\Config\Configuration;
use Momento\Config\ReadConcern;
use Momento\Config\Transport\StaticGrpcConfiguration;
use Momento\Config\Transport\StaticStorageTransportStrategy;
use Momento\Logging\ILoggerFactory;
use Momento\Logging\NullLoggerFactory;

/**
* Lambda config provides recommended configuration settings for a lambda environment.
*/
class Lambda extends Configuration
{
/**
* Provides the latest recommended configuration for a Lambda environment.
* This configuration may change in future releases to take advantage of
* improvements we identify for default configurations.
* @param ILoggerFactory|null $loggerFactory
* @return Lambda
*/
public static function latest(?ILoggerFactory $loggerFactory = null): Lambda
{
return self::v1($loggerFactory);
}

/**
* Provides the version 1 configuration for a Lambda environment. This configuration is guaranteed not to change
* in future releases of the SDK.
* @param ILoggerFactory|null $loggerFactory
* @return Lambda
*/
public static function v1(?ILoggerFactory $loggerFactory = null): Lambda
{
$loggerFactory = $loggerFactory ?? new NullLoggerFactory();
$grpcConfig = new StaticGrpcConfiguration(1100);
$transportStrategy = new StaticStorageTransportStrategy($grpcConfig, $loggerFactory, self::$maxIdleMillis);
Comment on lines +39 to +40
Copy link
Contributor

Choose a reason for hiding this comment

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

iirc, the main difference in the lambda config was that keepalives were turned off. should we also add the keepalive setting to the grpc config and turn them off here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, the keepalives were causing errors with infrequently invoked lambdas. If you want to add that in a future pr, we should get rid of the v1 config for now and only use latest.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good to know, thanks. I'll add that in.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I added the keepalive params into the gRPC config before realizing that keepalive is opt-in. The nodejs Lambda configuration just omits the keepalive parameters to use the gRPC defaults. So the Lambda config is good to go as-is and we're free to add keepalives to new configs and new versions of the existing configs in the future as we see fit.

$readConcern = ReadConcern::BALANCED;
return new Lambda($loggerFactory, $transportStrategy, $readConcern);
}
}
Loading