Skip to content

Commit 165b61f

Browse files
dpwatrousdpwatrous
authored
Use async iterator for node service list (#2907)
* Node service refactoring List API now uses async iterator. Also some naming changes, tests. * Add test for listing VM extensions to live-node-service
1 parent a10bb06 commit 165b61f

9 files changed

Lines changed: 397 additions & 92 deletions

File tree

‎packages/react/src/vm-extension/node-vm-ext-list.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export const NodeVMExtList = (props: NodeVmExtensionListProps) => {
2525
let isMounted = true;
2626
setLoading(true);
2727
nodeService
28-
.listBatchNodeExtensions(accountEndpoint, poolId, nodeId)
28+
.listVmExtensions(accountEndpoint, poolId, nodeId)
2929
.then((resList) => {
3030
if (!isMounted) {
3131
return;

‎packages/service/src/node/__tests__/fake-node-service.spec.ts‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@ import { BatchFakeSet, BasicBatchFakeSet } from "../../test-util/fakes";
33
import { FakeNodeService } from "../fake-node-service";
44

55
describe("FakeNodeService", () => {
6-
const poolId = `/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/supercomputing/providers/Microsoft.Batch/batchAccounts/hobo/pools/hobopool1`;
7-
const nodeId = `tvmps_id1`;
6+
const hoboAcctEndpoint = "mercury.eastus.batch.azure.com";
87

98
let service: FakeNodeService;
109
let fakeSet: BatchFakeSet;
@@ -17,18 +16,24 @@ describe("FakeNodeService", () => {
1716
});
1817

1918
test("List batch nodes", async () => {
20-
const nodes = await service.listBatchNodes("", poolId);
21-
expect(nodes?.map((node) => node.id)).toEqual([nodeId]);
19+
const nodes = await service.listNodes(hoboAcctEndpoint, "hobopool1");
20+
21+
const allNodes = [];
22+
for await (const node of nodes) {
23+
allNodes.push(node);
24+
}
25+
26+
expect(allNodes.map((node) => node.id)).toEqual(["tvmps_id1"]);
2227
});
2328

2429
test("List batch node extensions", async () => {
25-
const extensions = await service.listBatchNodeExtensions(
26-
"",
27-
"",
28-
nodeId
30+
const extensions = await service.listVmExtensions(
31+
hoboAcctEndpoint,
32+
"hobopool1",
33+
"tvmps_id1"
2934
);
3035
expect(
31-
extensions?.map((extension) => extension?.vmExtension?.name)
36+
extensions.map((extension) => extension?.vmExtension?.name)
3237
).toEqual(["CustomExtension100"]);
3338
});
3439
});

‎packages/service/src/node/__tests__/live-node-service.spec.ts‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
import { getMockEnvironment } from "@azure/bonito-core/lib/environment";
2+
import { MockHttpClient, MockHttpResponse } from "@azure/bonito-core/lib/http";
3+
import { BatchApiVersion } from "../../constants";
4+
import { initMockBatchEnvironment } from "../../environment";
5+
import { BasicBatchFakeSet, BatchFakeSet } from "../../test-util/fakes";
6+
import { LiveNodeService } from "../live-node-service";
7+
import { NodeService } from "../node-service";
8+
9+
describe("LiveNodeService", () => {
10+
const hoboAcctEndpoint = "mercury.eastus.batch.azure.com";
11+
12+
let service: NodeService;
13+
let fakeSet: BatchFakeSet;
14+
15+
let httpClient: MockHttpClient;
16+
17+
beforeEach(() => {
18+
initMockBatchEnvironment();
19+
httpClient = getMockEnvironment().getHttpClient();
20+
service = new LiveNodeService();
21+
fakeSet = new BasicBatchFakeSet();
22+
});
23+
24+
test("Simple get", async () => {
25+
httpClient.addExpected(
26+
new MockHttpResponse(
27+
`https://${hoboAcctEndpoint}/pools/hobopool1/nodes/tvmps_id1?api-version=${BatchApiVersion.data}`,
28+
{
29+
status: 200,
30+
body: JSON.stringify(
31+
fakeSet.getNode(
32+
hoboAcctEndpoint,
33+
"hobopool1",
34+
"tvmps_id1"
35+
)
36+
),
37+
}
38+
)
39+
);
40+
41+
const node = await service.getNode(
42+
hoboAcctEndpoint,
43+
"hobopool1",
44+
"tvmps_id1"
45+
);
46+
expect(node).toBeDefined();
47+
expect(node.id).toEqual("tvmps_id1");
48+
});
49+
50+
test("List by pool", async () => {
51+
httpClient.addExpected(
52+
new MockHttpResponse(
53+
`https://${hoboAcctEndpoint}/pools/hobopool1/nodes?api-version=${BatchApiVersion.data}`,
54+
{
55+
status: 200,
56+
body: JSON.stringify({
57+
value: fakeSet.listNodes(hoboAcctEndpoint, "hobopool1"),
58+
}),
59+
}
60+
)
61+
);
62+
63+
const nodes = await service.listNodes(hoboAcctEndpoint, "hobopool1");
64+
const allNodes = [];
65+
for await (const node of nodes) {
66+
allNodes.push(node);
67+
}
68+
expect(allNodes.length).toBe(1);
69+
expect(allNodes.map((node) => node.id)).toEqual(["tvmps_id1"]);
70+
});
71+
72+
test("List VM extensions", async () => {
73+
httpClient.addExpected(
74+
new MockHttpResponse(
75+
`https://${hoboAcctEndpoint}/pools/hobopool1/nodes/tvmps_id1/extensions?api-version=${BatchApiVersion.data}`,
76+
{
77+
status: 200,
78+
body: JSON.stringify({
79+
value: fakeSet.listVmExtensions("tvmps_id1"),
80+
}),
81+
}
82+
)
83+
);
84+
85+
const extensions = await service.listVmExtensions(
86+
hoboAcctEndpoint,
87+
"hobopool1",
88+
"tvmps_id1"
89+
);
90+
expect(extensions.length).toBe(1);
91+
expect(extensions.map((ext) => ext.vmExtension?.name)).toEqual([
92+
"CustomExtension100",
93+
]);
94+
});
95+
});

‎packages/service/src/node/fake-node-service.ts‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import { OperationOptions } from "@azure/bonito-core";
22
import { BatchFakeSet, BasicBatchFakeSet } from "../test-util/fakes";
33
import { BatchNodeOutput, BatchNodeVMExtensionOutput } from "./node-models";
4-
import type { NodeService } from "./node-service";
4+
import type { ListNodesOptions, NodeService } from "./node-service";
5+
import { PagedAsyncIterableIterator } from "@azure/core-paging";
6+
import { createPagedArray } from "../test-util/paging-test-util";
57

68
export class FakeNodeService implements NodeService {
79
fakeSet: BatchFakeSet = new BasicBatchFakeSet();
@@ -10,20 +12,31 @@ export class FakeNodeService implements NodeService {
1012
this.fakeSet = fakeSet;
1113
}
1214

13-
async listBatchNodes(
15+
async getNode(
1416
accountEndpoint: string,
15-
poolId: string,
16-
opts?: OperationOptions | undefined
17-
): Promise<BatchNodeOutput[] | undefined> {
18-
return this.fakeSet.listBatchNodes(poolId);
17+
poolName: string,
18+
nodeId: string,
19+
opts?: OperationOptions
20+
): Promise<BatchNodeOutput> {
21+
return this.fakeSet.getNode(accountEndpoint, poolName, nodeId);
22+
}
23+
24+
async listNodes(
25+
accountEndpoint: string,
26+
poolName: string,
27+
opts?: ListNodesOptions
28+
): Promise<PagedAsyncIterableIterator<BatchNodeOutput>> {
29+
return createPagedArray(
30+
this.fakeSet.listNodes(accountEndpoint, poolName)
31+
);
1932
}
2033

21-
async listBatchNodeExtensions(
34+
async listVmExtensions(
2235
accountEndpoint: string,
23-
poolId: string,
36+
poolName: string,
2437
nodeId: string,
25-
opts?: OperationOptions | undefined
26-
): Promise<BatchNodeVMExtensionOutput[] | undefined> {
27-
return this.fakeSet.listBatchNodeExtensions(nodeId);
38+
opts?: OperationOptions
39+
): Promise<BatchNodeVMExtensionOutput[]> {
40+
return this.fakeSet.listVmExtensions(nodeId);
2841
}
2942
}

‎packages/service/src/node/live-node-service.ts‎

Lines changed: 44 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
import { AbstractHttpService, OperationOptions } from "@azure/bonito-core";
22
import { BatchNodeOutput, BatchNodeVMExtensionOutput } from "./node-models";
3-
import { NodeService } from "./node-service";
4-
import { createBatchClient, isUnexpected } from "../internal/batch-rest";
3+
import { ListNodesOptions, NodeService } from "./node-service";
4+
import {
5+
createBatchClient,
6+
isUnexpected,
7+
paginate,
8+
} from "../internal/batch-rest";
59
import { createBatchUnexpectedStatusCodeError } from "../utils";
10+
import { PagedAsyncIterableIterator } from "@azure/core-paging";
611

712
export class LiveNodeService
813
extends AbstractHttpService
@@ -16,43 +21,68 @@ export class LiveNodeService
1621
return accountEndpoint;
1722
}
1823

19-
async listBatchNodes(
24+
async getNode(
2025
accountEndpoint: string,
21-
poolId: string,
22-
opts?: OperationOptions | undefined
23-
): Promise<BatchNodeOutput[] | undefined> {
26+
poolName: string,
27+
nodeId: string,
28+
opts?: OperationOptions
29+
): Promise<BatchNodeOutput> {
30+
const batchClient = createBatchClient(
31+
this._ensureHttpsEndpoint(accountEndpoint)
32+
);
33+
34+
const res = await batchClient
35+
.path("/pools/{poolId}/nodes/{nodeId}", poolName, nodeId)
36+
.get();
37+
38+
if (isUnexpected(res)) {
39+
throw createBatchUnexpectedStatusCodeError(res);
40+
}
41+
42+
return res.body;
43+
}
44+
45+
async listNodes(
46+
accountEndpoint: string,
47+
poolName: string,
48+
opts?: ListNodesOptions
49+
): Promise<PagedAsyncIterableIterator<BatchNodeOutput>> {
2450
const listNodePath = "/pools/{poolId}/nodes";
2551
const batchClient = createBatchClient(
2652
this._ensureHttpsEndpoint(accountEndpoint)
2753
);
28-
const res = await batchClient.path(listNodePath, poolId).get();
54+
const res = await batchClient.path(listNodePath, poolName).get({
55+
queryParameters: {
56+
$filter: opts?.filter,
57+
},
58+
});
2959

3060
if (isUnexpected(res)) {
3161
throw createBatchUnexpectedStatusCodeError(res);
3262
}
3363

34-
return res.body.value;
64+
return paginate(batchClient, res);
3565
}
3666

37-
async listBatchNodeExtensions(
67+
async listVmExtensions(
3868
accountEndpoint: string,
39-
poolId: string,
69+
poolName: string,
4070
nodeId: string,
41-
opts?: OperationOptions | undefined
42-
): Promise<BatchNodeVMExtensionOutput[] | undefined> {
71+
opts?: OperationOptions
72+
): Promise<BatchNodeVMExtensionOutput[]> {
4373
const listNodeExtensionPath =
4474
"/pools/{poolId}/nodes/{nodeId}/extensions";
4575
const batchClient = createBatchClient(
4676
this._ensureHttpsEndpoint(accountEndpoint)
4777
);
4878
const res = await batchClient
49-
.path(listNodeExtensionPath, poolId, nodeId)
79+
.path(listNodeExtensionPath, poolName, nodeId)
5080
.get();
5181

5282
if (isUnexpected(res)) {
5383
throw createBatchUnexpectedStatusCodeError(res);
5484
}
5585

56-
return res.body.value;
86+
return res.body.value ?? [];
5787
}
5888
}

‎packages/service/src/node/node-service.ts‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,29 @@
11
import { OperationOptions } from "@azure/bonito-core";
22
import { BatchNodeOutput, BatchNodeVMExtensionOutput } from "./node-models";
3+
import { PagedAsyncIterableIterator } from "@azure/core-paging";
4+
5+
export interface ListNodesOptions extends OperationOptions {
6+
filter: string;
7+
}
38

49
export interface NodeService {
5-
listBatchNodes(
10+
getNode(
611
accountEndpoint: string,
7-
poolId: string,
12+
poolName: string,
13+
nodeId: string,
814
opts?: OperationOptions
9-
): Promise<BatchNodeOutput[] | undefined>;
15+
): Promise<BatchNodeOutput>;
16+
17+
listNodes(
18+
accountEndpoint: string,
19+
poolName: string,
20+
opts?: ListNodesOptions
21+
): Promise<PagedAsyncIterableIterator<BatchNodeOutput>>;
1022

11-
listBatchNodeExtensions(
23+
listVmExtensions(
1224
accountEndpoint: string,
13-
poolId: string,
25+
poolName: string,
1426
nodeId: string,
1527
opts?: OperationOptions
16-
): Promise<BatchNodeVMExtensionOutput[] | undefined>;
28+
): Promise<BatchNodeVMExtensionOutput[]>;
1729
}

‎packages/service/src/test-util/__tests__/paging-test-util.spec.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import { PagedAsyncIterableIterator } from "@azure/core-paging";
2+
import { createPagedArray } from "../paging-test-util";
3+
4+
describe("createPagedArray function", () => {
5+
let result: PagedAsyncIterableIterator<string>;
6+
7+
beforeEach(() => {
8+
result = createPagedArray([
9+
"one",
10+
"two",
11+
"three",
12+
"four",
13+
"five",
14+
"six",
15+
"seven",
16+
]);
17+
});
18+
19+
test("Can be used as iterable", async () => {
20+
const resultFromIterable = [];
21+
for await (const num of result) {
22+
resultFromIterable.push(num);
23+
}
24+
expect(resultFromIterable.join(", ")).toEqual(
25+
"one, two, three, four, five, six, seven"
26+
);
27+
});
28+
29+
test("Can be used as iterator", async () => {
30+
expect(await result.next()).toEqual({ value: "one", done: false });
31+
expect(await result.next()).toEqual({ value: "two", done: false });
32+
expect(await result.next()).toEqual({ value: "three", done: false });
33+
expect(await result.next()).toEqual({ value: "four", done: false });
34+
expect(await result.next()).toEqual({ value: "five", done: false });
35+
expect(await result.next()).toEqual({ value: "six", done: false });
36+
expect(await result.next()).toEqual({ value: "seven", done: false });
37+
expect(await result.next()).toEqual({ value: undefined, done: true });
38+
});
39+
40+
test("Can iterate by page", async () => {
41+
const pages = result.byPage();
42+
expect((await pages.next()).value.join(", ")).toEqual(
43+
"one, two, three, four, five"
44+
);
45+
expect((await pages.next()).value.join(", ")).toEqual("six, seven");
46+
expect(await pages.next()).toEqual({
47+
value: undefined,
48+
done: true,
49+
});
50+
});
51+
});

0 commit comments

Comments
 (0)