Skip to content

Commit 67edcd3

Browse files
vihangmcopybaranaut
authored andcommitted
Use ClusterConfig everywhere instead of overloading with clusterID
Summary: This tidies things up a bit and makes it a bit more explicit. Test Plan: yarn typecheck works, yarn test works, yarn dev + load scripts, works. Reviewers: michelle, nlanam Reviewed By: nlanam Signed-off-by: Vihang Mehta <vihang@pixielabs.ai> Differential Revision: https://phab.corp.pixielabs.ai/D11738 GitOrigin-RevId: ddcf9d9
1 parent 75f8e19 commit 67edcd3

3 files changed

Lines changed: 15 additions & 19 deletions

File tree

src/ui/src/api/api-test.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ describe('Pixie TypeScript API Client', () => {
8080
jest.spyOn(client.getCloudClient(), 'getClusterConnection')
8181
.mockReturnValue({} as unknown as Promise<GQLClusterConnectionInfo>);
8282

83-
const out = await client.health('foo').toPromise();
83+
const out = await client.health({ id: 'foo', passthroughClusterAddress: '' }).toPromise();
8484
expect(spy).toHaveBeenCalled();
8585
expect(out).toBe('bar');
8686
});
@@ -93,7 +93,11 @@ describe('Pixie TypeScript API Client', () => {
9393
jest.spyOn(client.getCloudClient(), 'getClusterConnection')
9494
.mockReturnValue({} as unknown as Promise<GQLClusterConnectionInfo>);
9595

96-
const out = await client.executeScript('foo', 'import px', { enableE2EEncryption: false }).toPromise();
96+
const out = await client.executeScript(
97+
{ id: 'foo', passthroughClusterAddress: '' },
98+
'import px',
99+
{ enableE2EEncryption: false },
100+
).toPromise();
97101
expect(spy).toHaveBeenCalled();
98102
expect(out).toBe('bar');
99103
});

src/ui/src/api/api.ts

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,10 @@ export abstract class PixieAPIClientAbstract {
5757

5858
readonly options: Required<PixieAPIClientOptions>;
5959

60-
abstract health(cluster: string | ClusterConfig): Observable<Status>;
60+
abstract health(cluster: ClusterConfig): Observable<Status>;
6161

6262
abstract executeScript(
63-
cluster: string | ClusterConfig,
63+
cluster: ClusterConfig,
6464
script: string,
6565
opts: ExecuteScriptOptions,
6666
funcs?: VizierQueryFunc[],
@@ -134,20 +134,12 @@ export class PixieAPIClient extends PixieAPIClientAbstract {
134134
return client;
135135
}
136136

137-
private async getClusterClient(cluster: string | ClusterConfig) {
138-
let id: string;
139-
let passthroughClusterAddress: string;
140-
let attachCredentials = false;
141-
142-
if (typeof cluster === 'string') {
143-
id = cluster;
144-
} else {
145-
({ id, passthroughClusterAddress, attachCredentials } = cluster);
146-
}
137+
private async getClusterClient(cluster: ClusterConfig) {
138+
const { id } = cluster;
147139

148140
return this.clusterConnections.has(id)
149141
? Promise.resolve(this.clusterConnections.get(id))
150-
: this.createVizierClient({ id, passthroughClusterAddress, attachCredentials });
142+
: this.createVizierClient(cluster);
151143
}
152144

153145
// TODO(nick): Once the authentication model settles down, make this easier to use outside of the browser.
@@ -174,7 +166,7 @@ export class PixieAPIClient extends PixieAPIClientAbstract {
174166
* @param cluster Which cluster to use. Either just its ID, or a full config. If that cluster has previously been
175167
* connected in this session, that connection will be reused without changing its configuration.
176168
*/
177-
health(cluster: string | ClusterConfig): Observable<Status> {
169+
health(cluster: ClusterConfig): Observable<Status> {
178170
return from(this.getClusterClient(cluster))
179171
.pipe(switchMap((client) => client.health()));
180172
}
@@ -196,7 +188,7 @@ export class PixieAPIClient extends PixieAPIClientAbstract {
196188
* @param funcs Descriptions of which functions in the script to run, and what to do with their output.
197189
*/
198190
executeScript(
199-
cluster: string | ClusterConfig,
191+
cluster: ClusterConfig,
200192
script: string,
201193
opts: ExecuteScriptOptions,
202194
funcs: VizierQueryFunc[] = [],

src/ui/src/testing/mocks/api-mock.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,13 @@ export class MockPixieAPIClient implements PixieAPIClientAbstract {
3737

3838
// eslint-disable-next-line class-methods-use-this
3939
executeScript(
40-
cluster: string | ClusterConfig, script: string, opts: ExecuteScriptOptions, funcs?: VizierQueryFunc[],
40+
cluster: ClusterConfig, script: string, opts: ExecuteScriptOptions, funcs?: VizierQueryFunc[],
4141
): Observable<ExecutionStateUpdate> {
4242
return observableOf({} as ExecutionStateUpdate);
4343
}
4444

4545
// eslint-disable-next-line class-methods-use-this
46-
health(cluster: string | ClusterConfig): Observable<Status> {
46+
health(cluster: ClusterConfig): Observable<Status> {
4747
return observableOf(new Status().setCode(0));
4848
}
4949

0 commit comments

Comments
 (0)