Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove SummarizerStopReason, ISummarizeEventProps, and ISummarizerEvents #23483

Merged
merged 2 commits into from
Jan 9, 2025
Merged

Remove SummarizerStopReason, ISummarizeEventProps, and ISummarizerEvents #23483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d4b4897
Remove SummarizerStopReason, ISummarizeEventProps, and ISummarizerEvents
kian-thompson Jan 7, 2025
0a25c7b
Merge branch 'main' into 26524-remove-deprecated-summmarizer-types
kian-thompson Jan 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changeset/sweet-tires-bathe.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"@fluidframework/container-runtime": minor
---
---
"section": legacy
---

Removed SummarizerStopReason, ISummarizeEventProps, and ISummarizerEvents

`SummarizerStopReason`, `ISummarizeEventProps`, and `ISummarizerEvents` have all been removed from the `"@fluidframework/container-runtime"` package. Please migrate all uses of these APIs to their respective copies in the `"@fluidframework/container-runtime-definitions"` package.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jason-ha was suggesting we link to the release note where the deprecation was announced, rather than having to restate everything again in duplicate.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep - but okay if the migration notes are short like this.

63 changes: 9 additions & 54 deletions ...ages/runtime/container-runtime/api-report/container-runtime.legacy.alpha.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ export interface IBroadcastSummaryResult {
// @alpha
export interface ICancellableSummarizerController extends ISummaryCancellationToken {
// (undocumented)
stop(reason: SummarizerStopReason_2): void;
stop(reason: SummarizerStopReason): void;
}

// @alpha
Expand Down Expand Up @@ -389,33 +389,21 @@ export interface ISubmitSummaryOptions extends ISummarizeOptions {
readonly summaryLogger: ITelemetryLoggerExt;
}

// @alpha @deprecated (undocumented)
export interface ISummarizeEventProps {
// (undocumented)
currentAttempt: number;
// (undocumented)
error?: any;
// (undocumented)
maxAttempts: number;
// (undocumented)
result: "success" | "failure" | "canceled";
}

// @alpha
export interface ISummarizeOptions {
readonly fullTree?: boolean;
}

// @alpha (undocumented)
export interface ISummarizer extends IEventProvider<ISummarizerEvents_2> {
export interface ISummarizer extends IEventProvider<ISummarizerEvents> {
// (undocumented)
close(): void;
enqueueSummarize(options: IEnqueueSummarizeOptions): EnqueueSummarizeResult;
readonly ISummarizer?: ISummarizer;
// (undocumented)
run(onBehalfOf: string): Promise<SummarizerStopReason_2>;
run(onBehalfOf: string): Promise<SummarizerStopReason>;
// (undocumented)
stop(reason: SummarizerStopReason_2): void;
stop(reason: SummarizerStopReason): void;
summarizeOnDemand(options: IOnDemandSummarizeOptions): ISummarizeResults;
}

Expand All @@ -426,12 +414,6 @@ export interface ISummarizeResults {
readonly summarySubmitted: Promise<SummarizeResultPart<SubmitSummaryResult, SubmitSummaryFailureData>>;
}

// @alpha @deprecated (undocumented)
export interface ISummarizerEvents extends IEvent {
// (undocumented)
(event: "summarize", listener: (props: ISummarizeEventProps) => void): any;
}

// @alpha (undocumented)
export interface ISummarizerInternalsProvider {
refreshLatestSummaryAck(options: IRefreshSummaryAckOptions): Promise<void>;
Expand Down Expand Up @@ -483,7 +465,7 @@ export interface ISummaryBaseConfiguration {
}

// @alpha
export type ISummaryCancellationToken = ICancellationToken<SummarizerStopReason_2>;
export type ISummaryCancellationToken = ICancellationToken<SummarizerStopReason>;

// @alpha (undocumented)
export interface ISummaryCollectionOpEvents extends IEvent {
Expand Down Expand Up @@ -601,7 +583,7 @@ export interface SubmitSummaryFailureData {
export type SubmitSummaryResult = IBaseSummarizeResult | IGenerateSummaryTreeResult | IUploadSummaryResult | ISubmitSummaryOpResult;

// @alpha
export class Summarizer extends TypedEventEmitter<ISummarizerEvents_2> implements ISummarizer {
export class Summarizer extends TypedEventEmitter<ISummarizerEvents> implements ISummarizer {
constructor(
runtime: ISummarizerRuntime, configurationGetter: () => ISummaryConfiguration,
internalsProvider: ISummarizerInternalsProvider, handleContext: IFluidHandleContext, summaryCollection: SummaryCollection, runCoordinatorCreateFn: (runtime: IConnectableRuntime) => Promise<ICancellableSummarizerController>);
Expand All @@ -615,9 +597,9 @@ export class Summarizer extends TypedEventEmitter<ISummarizerEvents_2> implement
// (undocumented)
recordSummaryAttempt?(summaryRefSeqNum?: number): void;
// (undocumented)
run(onBehalfOf: string): Promise<SummarizerStopReason_2>;
stop(reason: SummarizerStopReason_2): void;
static stopReasonCanRunLastSummary(stopReason: SummarizerStopReason_2): boolean;
run(onBehalfOf: string): Promise<SummarizerStopReason>;
stop(reason: SummarizerStopReason): void;
static stopReasonCanRunLastSummary(stopReason: SummarizerStopReason): boolean;
// (undocumented)
summarizeOnDemand(options: IOnDemandSummarizeOptions): ISummarizeResults;
// (undocumented)
Expand All @@ -635,33 +617,6 @@ export type SummarizeResultPart<TSuccess, TFailure = undefined> = {
error: IRetriableFailureError;
};

// @alpha @deprecated (undocumented)
export type SummarizerStopReason =
/** Summarizer client failed to summarize in all attempts. */
"failToSummarize"
/** Parent client reported that it is no longer connected. */
| "parentNotConnected"
/**
* Parent client reported that it is no longer elected the summarizer.
* This is the normal flow; a disconnect will always trigger the parent
* client to no longer be elected as responsible for summaries. Then it
* tries to stop its spawned summarizer client.
*/
| "notElectedParent"
/**
* We are not already running the summarizer and we are not the current elected client id.
*/
| "notElectedClient"
/** Summarizer client was disconnected */
| "summarizerClientDisconnected"
/** running summarizer threw an exception */
| "summarizerException"
/**
* The previous summary state on the summarizer is not the most recently acked summary. this also happens when the
* first submitSummary attempt fails for any reason and there's a 2nd summary attempt without an ack
*/
| "latestSummaryStateStale";

// @alpha
export class SummaryCollection extends TypedEventEmitter<ISummaryCollectionOpEvents> {
constructor(deltaManager: IDeltaManager<ISequencedDocumentMessage, IDocumentMessage>, logger: ITelemetryLoggerExt);
Expand Down
12 changes: 12 additions & 0 deletions packages/runtime/container-runtime/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,18 @@
},
"ClassStatics_ContainerRuntime": {
"backCompat": false
},
"TypeAlias_SummarizerStopReason": {
"backCompat": false,
"forwardCompat": false
},
"Interface_ISummarizeEventProps": {
"backCompat": false,
"forwardCompat": false
},
"Interface_ISummarizerEvents": {
"backCompat": false,
"forwardCompat": false
}
},
"entrypoint": "legacy"
Expand Down
3 changes: 0 additions & 3 deletions packages/runtime/container-runtime/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ export {
ISummaryCancellationToken,
neverCancelledSummaryToken,
Summarizer,
SummarizerStopReason,
SummaryCollection,
EnqueueSummarizeResult,
IAckSummaryResult,
Expand All @@ -75,7 +74,6 @@ export {
ISubmitSummaryOptions,
ISerializedElection,
ISummarizeOptions,
ISummarizerEvents,
ISummarizerInternalsProvider,
ISummarizerRuntime,
ISummarizingWarning,
Expand All @@ -95,7 +93,6 @@ export {
SubmitSummaryFailureData,
SummaryStage,
IRetriableFailureError,
ISummarizeEventProps,
IdCompressorMode,
IDocumentSchema,
DocumentSchemaValueType,
Expand Down
3 changes: 0 additions & 3 deletions packages/runtime/container-runtime/src/summary/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,10 @@ export {
ISummarizeHeuristicData,
ISummarizer,
ISummarizeResults,
ISummarizerEvents,
ISummarizerInternalsProvider,
ISummarizerRuntime,
ISummaryCancellationToken,
SubmitSummaryResult,
SummarizerStopReason,
EnqueueSummarizeResult,
IAckSummaryResult,
IBaseSummarizeResult,
Expand All @@ -67,7 +65,6 @@ export {
SubmitSummaryFailureData,
SummaryStage,
IRetriableFailureError,
ISummarizeEventProps,
} from "./summarizerTypes.js";
export {
IAckedSummary,
Expand Down
65 changes: 6 additions & 59 deletions packages/runtime/container-runtime/src/summary/summarizerTypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@ import {
ContainerWarning,
} from "@fluidframework/container-definitions/internal";
import type {
ISummarizerEvents as NewISummarizerEvents,
SummarizerStopReason as NewSummarizerStopReason,
ISummarizerEvents,
SummarizerStopReason,
} from "@fluidframework/container-runtime-definitions/internal";
import {
IEvent,
IEventProvider,
ITelemetryBaseProperties,
ITelemetryBaseLogger,
Expand Down Expand Up @@ -58,7 +57,7 @@ export interface ICancellationToken<T> {
* @legacy
* @alpha
*/
export type ISummaryCancellationToken = ICancellationToken<NewSummarizerStopReason>;
export type ISummaryCancellationToken = ICancellationToken<SummarizerStopReason>;

/**
* Data required to update internal tracking state after receiving a Summary Ack.
Expand Down Expand Up @@ -401,60 +400,8 @@ export type EnqueueSummarizeResult =
/**
* @legacy
* @alpha
* @deprecated Use SummarizerStopReason from the "\@fluidframework/container-runtime-definitions" package
*/
export type SummarizerStopReason =
/** Summarizer client failed to summarize in all attempts. */
| "failToSummarize"
/** Parent client reported that it is no longer connected. */
| "parentNotConnected"
/**
* Parent client reported that it is no longer elected the summarizer.
* This is the normal flow; a disconnect will always trigger the parent
* client to no longer be elected as responsible for summaries. Then it
* tries to stop its spawned summarizer client.
*/
| "notElectedParent"
/**
* We are not already running the summarizer and we are not the current elected client id.
*/
| "notElectedClient"
/** Summarizer client was disconnected */
| "summarizerClientDisconnected"
/** running summarizer threw an exception */
| "summarizerException"
/**
* The previous summary state on the summarizer is not the most recently acked summary. this also happens when the
* first submitSummary attempt fails for any reason and there's a 2nd summary attempt without an ack
*/
| "latestSummaryStateStale";

/**
* @legacy
* @alpha
* @deprecated Use ISummarizeEventProps from the "\@fluidframework/container-runtime-definitions" package
*/
export interface ISummarizeEventProps {
result: "success" | "failure" | "canceled";
currentAttempt: number;
maxAttempts: number;
error?: any;
}

/**
* @legacy
* @alpha
* @deprecated Use ISummarizerEvents from the "\@fluidframework/container-runtime-definitions" package
*/
export interface ISummarizerEvents extends IEvent {
(event: "summarize", listener: (props: ISummarizeEventProps) => void);
}

/**
* @legacy
* @alpha
*/
export interface ISummarizer extends IEventProvider<NewISummarizerEvents> {
export interface ISummarizer extends IEventProvider<ISummarizerEvents> {
/**
* Allows {@link ISummarizer} to be used with our {@link @fluidframework/core-interfaces#FluidObject} pattern.
*/
Expand All @@ -465,12 +412,12 @@ export interface ISummarizer extends IEventProvider<NewISummarizerEvents> {
* Summarizer will finish current processes, which may take a while.
* For example, summarizer may complete last summary before exiting.
*/
stop(reason: NewSummarizerStopReason): void;
stop(reason: SummarizerStopReason): void;

/* Closes summarizer. Any pending processes (summary in flight) are abandoned. */
close(): void;

run(onBehalfOf: string): Promise<NewSummarizerStopReason>;
run(onBehalfOf: string): Promise<SummarizerStopReason>;

/**
* Attempts to generate a summary on demand. If already running, takes no action.
Expand Down
6 changes: 6 additions & 0 deletions ...es/runtime/container-runtime/src/test/types/validateContainerRuntimePrevious.generated.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,7 @@ declare type current_as_old_for_Interface_ISubmitSummaryOptions = requireAssigna
* typeValidation.broken:
* "Interface_ISummarizeEventProps": {"forwardCompat": false}
*/
// @ts-expect-error compatibility expected to be broken
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CraigMacomber -- I thought when something was removed, it got a special new entry in this file. Did that change?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type tests don't understand "removal". The tests are generated purely from the prior package API and the break settings of the current package.
The testing shouldn't care if the API still exists or not - just that the new API surface is (isn't) the same as the old.
(This was a nice change in strategy that Craig put in place.)

declare type old_as_current_for_Interface_ISummarizeEventProps = requireAssignableTo<TypeOnly<old.ISummarizeEventProps>, TypeOnly<current.ISummarizeEventProps>>

/*
Expand All @@ -753,6 +754,7 @@ declare type old_as_current_for_Interface_ISummarizeEventProps = requireAssignab
* typeValidation.broken:
* "Interface_ISummarizeEventProps": {"backCompat": false}
*/
// @ts-expect-error compatibility expected to be broken
declare type current_as_old_for_Interface_ISummarizeEventProps = requireAssignableTo<TypeOnly<current.ISummarizeEventProps>, TypeOnly<old.ISummarizeEventProps>>

/*
Expand Down Expand Up @@ -816,6 +818,7 @@ declare type current_as_old_for_Interface_ISummarizeResults = requireAssignableT
* typeValidation.broken:
* "Interface_ISummarizerEvents": {"forwardCompat": false}
*/
// @ts-expect-error compatibility expected to be broken
declare type old_as_current_for_Interface_ISummarizerEvents = requireAssignableTo<TypeOnly<old.ISummarizerEvents>, TypeOnly<current.ISummarizerEvents>>

/*
Expand All @@ -825,6 +828,7 @@ declare type old_as_current_for_Interface_ISummarizerEvents = requireAssignableT
* typeValidation.broken:
* "Interface_ISummarizerEvents": {"backCompat": false}
*/
// @ts-expect-error compatibility expected to be broken
declare type current_as_old_for_Interface_ISummarizerEvents = requireAssignableTo<TypeOnly<current.ISummarizerEvents>, TypeOnly<old.ISummarizerEvents>>

/*
Expand Down Expand Up @@ -1428,6 +1432,7 @@ declare type current_as_old_for_TypeAlias_SummarizeResultPart = requireAssignabl
* typeValidation.broken:
* "TypeAlias_SummarizerStopReason": {"forwardCompat": false}
*/
// @ts-expect-error compatibility expected to be broken
declare type old_as_current_for_TypeAlias_SummarizerStopReason = requireAssignableTo<TypeOnly<old.SummarizerStopReason>, TypeOnly<current.SummarizerStopReason>>

/*
Expand All @@ -1437,6 +1442,7 @@ declare type old_as_current_for_TypeAlias_SummarizerStopReason = requireAssignab
* typeValidation.broken:
* "TypeAlias_SummarizerStopReason": {"backCompat": false}
*/
// @ts-expect-error compatibility expected to be broken
declare type current_as_old_for_TypeAlias_SummarizerStopReason = requireAssignableTo<TypeOnly<current.SummarizerStopReason>, TypeOnly<old.SummarizerStopReason>>

/*
Expand Down
Loading