feat: wrapper client setKeepAlive method #339
Conversation
crystall-bitquill
added
the
ready for review
| @@ -26,6 +26,7 @@ import { HostInfo } from "../../../common/lib/host_info"; | |||
|
|
|||
| export class MySQL2DriverDialect implements DriverDialect { | |||
| protected dialectName: string = this.constructor.name; | |||
| keepAlivePropertyNames: string[] = []; | |||
There was a problem hiding this comment.
Is there a reason this is not a static constant? can this be modified?
There was a problem hiding this comment.
Why the type is string[], not just string?
There was a problem hiding this comment.
It's a string[] because there could be multiple properties, for example PG having both keepalive and keepaliveInitialDelayMillis
crystall-bitquill
force-pushed
the
feat/tcp-keepalive
branch
2 times, most recently
from
5fe5632 to
abe26ca
Compare
| @@ -52,4 +52,6 @@ export class MySQL2DriverDialect implements DriverDialect { | |||
| getAwsPoolClient(props: PoolOptions): AwsPoolClient { | |||
| return new AwsMysqlPoolClient(props); | |||
| } | |||
|
|
|||
| setupInitialProperties(props: Map<string, any>) {} | |||
There was a problem hiding this comment.
Should we notify user that this feature isn't supported by mysql driver if the user provides tcpKeepAlive configuration?
There was a problem hiding this comment.
I've added a note in the UsingTheNodejsWrapper.md file, do you think that's sufficient?
common/lib/aws_client.ts
Outdated
| @@ -58,6 +58,8 @@ export abstract class AwsClient extends EventEmitter { | |||
|
|
|||
| this.properties = new Map<string, any>(Object.entries(config)); | |||
|
|
|||
| driverDialect.setupInitialProperties(this.properties); | |||
There was a problem hiding this comment.
I think it's better to setup these properties in connection provider connect() method.
There was a problem hiding this comment.
or in driver dialect connect()
crystall-bitquill
force-pushed
the
feat/tcp-keepalive
branch
3 times, most recently
from
970ac66 to
33b1444
Compare
karenc-bq
force-pushed
the
feat/tcp-keepalive
branch
from
33b1444 to
0734c4c
Compare
karenc-bq
force-pushed
the
feat/tcp-keepalive
branch
from
6fc9df1 to
c9b9aa1
Compare
| @@ -52,4 +52,6 @@ export class MySQL2DriverDialect implements DriverDialect { | |||
| getAwsPoolClient(props: PoolOptions): AwsPoolClient { | |||
| return new AwsMysqlPoolClient(props); | |||
| } | |||
|
|
|||
| setKeepAliveProperties(props: Map<string, any>, keepAliveProps: any) {} | |||
There was a problem hiding this comment.
Since mysql driver doesn't support tcpKeepAlive I'd suggest to check keepAliveProp and if it set then raise an exception saying that the driver doesn't support it.
Summary
Wrapper client setKeepAlive method
Description
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.