Skip to content

Commit 2cba272

Browse files
committed
review comments
1 parent 40e715f commit 2cba272

3 files changed

Lines changed: 28 additions & 30 deletions

File tree

handwritten/spanner/google-cloud-spanner-executor/src/cloud-client-executor.ts

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,22 @@ export class ExecutionFlowContext implements ExecutionFlowContextInterface {
8484
}
8585
}
8686

87+
type ActionHandler = (action: any, sender: OutcomeSender) => Promise<void>;
88+
8789
export class CloudClientExecutor {
8890
private spanner: Spanner;
8991
private tracer: Tracer;
9092

93+
private readonly adminActionRegistry: Record<string, ActionHandler> = {
94+
createCloudInstance: (action, sender) =>
95+
this.executeCreateCloudInstance(action as ICreateCloudInstanceAction, sender),
96+
};
97+
98+
private readonly actionRegistry: Record<string, ActionHandler> = {
99+
admin: (action, sender) =>
100+
this.executeAdminAction(action as IAdminAction, sender),
101+
};
102+
91103
constructor() {
92104
const spannerOptions = CloudUtil.getSpannerOptions();
93105
this.spanner = new Spanner(spannerOptions);
@@ -144,8 +156,9 @@ export class CloudClientExecutor {
144156

145157
return context.with(trace.setSpan(context.active(), span), async () => {
146158
try {
147-
if (action.admin) {
148-
await this.executeAdminAction(action.admin, outcomeSender);
159+
const handler = this.actionRegistry[actionType];
160+
if (handler) {
161+
await handler(action[actionType as keyof typeof action], outcomeSender);
149162
return;
150163
}
151164

@@ -171,16 +184,21 @@ export class CloudClientExecutor {
171184
sender: OutcomeSender,
172185
): Promise<void> {
173186
try {
174-
if (action.createCloudInstance) {
175-
await this.executeCreateCloudInstance(
176-
action.createCloudInstance,
187+
const adminType = Object.keys(action).find(
188+
k => action[k as keyof typeof action] !== undefined,
189+
);
190+
191+
if (adminType && this.adminActionRegistry[adminType]) {
192+
await this.adminActionRegistry[adminType](
193+
action[adminType as keyof typeof action],
177194
sender,
178195
);
179196
return;
180197
}
198+
181199
sender.finishWithError({
182200
code: status.UNIMPLEMENTED,
183-
message: 'Admin action not implemented',
201+
message: `Admin action ${adminType || 'unknown'} not implemented`,
184202
});
185203
} catch (e: any) {
186204
sender.finishWithError(e);
@@ -213,7 +231,6 @@ export class CloudClientExecutor {
213231
});
214232

215233
console.log('Waiting for instance creation operation to complete...');
216-
217234
await operation.promise();
218235

219236
console.log(`Instance ${instanceId} created successfully.`);

handwritten/spanner/google-cloud-spanner-executor/src/cloud-executor-impl.ts

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
import {ServerDuplexStream, status, ServiceError} from '@grpc/grpc-js';
17+
import { ServerDuplexStream, status } from '@grpc/grpc-js';
1818
import {trace, context, Tracer} from '@opentelemetry/api';
1919
import {CloudClientExecutor} from './cloud-client-executor';
2020
import * as protos from '../../protos/protos';
@@ -64,22 +64,7 @@ export class CloudExecutorImpl {
6464
call.on('data', (request: SpannerAsyncActionRequest) => {
6565
context.with(streamContext, () => {
6666
console.log(`Receiving request: \n${JSON.stringify(request, null, 2)}`);
67-
68-
// Ensure nested properties exist before attempting to inject configuration overrides.
69-
if (!request.action) request.action = {};
70-
if (!request.action.spannerOptions) request.action.spannerOptions = {};
71-
if (!request.action.spannerOptions.sessionPoolOptions)
72-
request.action.spannerOptions.sessionPoolOptions = {};
73-
74-
// as the multiplexed session is defualt enabled, mutate the incoming request to forcefully enforce multiplexed sessions for this proxy execution.
75-
request.action.spannerOptions.sessionPoolOptions.useMultiplexed = true;
76-
77-
console.log(
78-
`Updated request to set multiplexed session flag: \n${JSON.stringify(request, null, 2)}`,
79-
);
80-
8167
// TODO: Set requestHasReadOrQueryAction flag here when Read/Query are implemented.
82-
8368
try {
8469
const reqStatus = this.clientExecutor.startHandlingRequest(
8570
request,

handwritten/spanner/google-cloud-spanner-executor/src/worker-proxy.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,7 @@ const OPTION_CERTIFICATE = 'cert';
4242
const OPTION_SERVICE_KEY_FILE = 'service_key_file';
4343
const OPTION_USE_PLAIN_TEXT_CHANNEL = 'use_plain_text_channel';
4444
const OPTION_ENABLE_GRPC_FAULT_INJECTOR = 'enable_grpc_fault_injector';
45-
const OPTION_MULTIPLEXED_SESSION_OPERATIONS_RATIO =
46-
'multiplexed_session_operations_ratio';
45+
4746

4847
/**
4948
* Acts as a proxy server that forwards incoming gRPC requests to the underlying
@@ -54,7 +53,7 @@ export class WorkerProxy {
5453
public static proxyPort = 0;
5554
public static cert = '';
5655
public static serviceKeyFile = '';
57-
public static multiplexedSessionOperationsRatio = 0.0;
56+
5857
public static usePlainTextChannel = false;
5958
public static enableGrpcFaultInjector = false;
6059
public static openTelemetrySdk: NodeTracerProvider;
@@ -126,10 +125,7 @@ export class WorkerProxy {
126125
type: 'boolean',
127126
description: 'Enable grpc fault injector in cloud client executor.',
128127
});
129-
parser.option(OPTION_MULTIPLEXED_SESSION_OPERATIONS_RATIO, {
130-
type: 'number',
131-
description: 'Ratio of operations to use multiplexed sessions.',
132-
});
128+
133129

134130
try {
135131
return parser.parseSync();

0 commit comments

Comments
 (0)