Skip to content

Commit 6cc743c

Browse files
committed
feedback
1 parent 00df0ed commit 6cc743c

5 files changed

Lines changed: 37 additions & 6 deletions

File tree

packages/durabletask-js-export-history/src/client/export-history-client.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,13 +211,13 @@ export class ExportHistoryJobClient {
211211

212212
/**
213213
* Deletes the export job.
214-
* Signals entity deletion via operation orchestrator (fire-and-forget, aligned with .NET),
214+
* Schedules entity deletion via operation orchestrator (does not wait for completion, aligned with .NET),
215215
* then terminates and purges the linked export orchestration.
216216
*/
217217
async delete(): Promise<void> {
218218
const orchestrationInstanceId = getOrchestratorInstanceId(this.jobId);
219219

220-
// First, delete the entity (fire-and-forget — does not wait for the operation orchestrator)
220+
// Schedule entity deletion (does not wait for the operation orchestrator to complete)
221221
const request: ExportJobOperationRequest = {
222222
entityId: this.entityId.toString(),
223223
operationName: "delete",

packages/durabletask-js-export-history/src/entity/export-job.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,10 @@ export class ExportJob extends TaskEntity<ExportJobState> {
152152
this.state.lastCheckpointTime = now;
153153
this.state.lastModifiedAt = now;
154154

155-
// If there are failures and checkpoint is null (batch failed), mark job as failed
155+
// Record failures for diagnostics but do NOT update job status here.
156+
// Failure status transitions are handled exclusively by markAsFailed(),
157+
// which is called by the orchestrator after retry exhaustion.
156158
if (!request.checkpoint && request.failures && request.failures.length > 0) {
157-
this.state.status = ExportJobStatus.Failed;
158159
const failureSummary = request.failures
159160
.map((f) => `${f.instanceId}: ${f.reason}`)
160161
.join("; ");

packages/durabletask-js-export-history/src/models/export-job-creation-options.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,15 @@ export function createExportJobCreationOptions(
147147
? options.runtimeStatus
148148
: terminalStatuses;
149149

150+
// Validate maxParallelExports range
151+
const validatedMaxParallelExports = options.maxParallelExports ?? DEFAULT_MAX_PARALLEL_EXPORTS;
152+
if (validatedMaxParallelExports <= 0) {
153+
throw new ExportJobClientValidationError(
154+
"MaxParallelExports must be greater than 0.",
155+
"maxParallelExports",
156+
);
157+
}
158+
150159
return {
151160
jobId,
152161
mode,
@@ -156,6 +165,6 @@ export function createExportJobCreationOptions(
156165
destination: options.destination,
157166
format: options.format ?? createExportFormat(ExportFormatKind.Jsonl),
158167
maxInstancesPerBatch: options.maxInstancesPerBatch ?? DEFAULT_MAX_INSTANCES_PER_BATCH,
159-
maxParallelExports: options.maxParallelExports ?? DEFAULT_MAX_PARALLEL_EXPORTS,
168+
maxParallelExports: validatedMaxParallelExports,
160169
};
161170
}

packages/durabletask-js-export-history/test/models.spec.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,27 @@ describe("Models", () => {
198198
).toThrow("MaxInstancesPerBatch");
199199
});
200200

201+
it("should throw when maxParallelExports is zero or negative", () => {
202+
expect(() =>
203+
createExportJobCreationOptions({
204+
jobId: "test-job",
205+
completedTimeFrom: new Date("2024-01-01"),
206+
completedTimeTo: new Date("2024-06-01"),
207+
mode: ExportMode.Batch,
208+
maxParallelExports: 0,
209+
}),
210+
).toThrow("MaxParallelExports");
211+
expect(() =>
212+
createExportJobCreationOptions({
213+
jobId: "test-job",
214+
completedTimeFrom: new Date("2024-01-01"),
215+
completedTimeTo: new Date("2024-06-01"),
216+
mode: ExportMode.Batch,
217+
maxParallelExports: -1,
218+
}),
219+
).toThrow("MaxParallelExports");
220+
});
221+
201222
it("should throw when runtimeStatus contains non-terminal status", () => {
202223
expect(() =>
203224
createExportJobCreationOptions({

test/e2e-azuremanaged/export-history.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) Microsoft Corporation. All rights reserved.
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

44
/**

0 commit comments

Comments
 (0)