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.

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
Changes from 1 commit
Commits
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
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.

Original file line number Diff line number Diff line change
@@ -287,7 +287,7 @@ export interface IBroadcastSummaryResult {
// @alpha
export interface ICancellableSummarizerController extends ISummaryCancellationToken {
// (undocumented)
stop(reason: SummarizerStopReason_2): void;
stop(reason: SummarizerStopReason): void;
}

// @alpha
@@ -540,33 +540,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;
}

@@ -577,12 +565,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>;
@@ -634,7 +616,7 @@ export interface ISummaryBaseConfiguration {
}

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

// @alpha (undocumented)
export interface ISummaryCollectionOpEvents extends IEvent {
@@ -752,7 +734,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>);
@@ -766,9 +748,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)
@@ -786,33 +768,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);
15 changes: 14 additions & 1 deletion packages/runtime/container-runtime/package.json
Original file line number Diff line number Diff line change
@@ -231,7 +231,20 @@
"typescript": "~5.4.5"
},
"typeValidation": {
"broken": {},
"broken": {
"TypeAlias_SummarizerStopReason": {
"backCompat": false,
"forwardCompat": false
},
"Interface_ISummarizeEventProps": {
"backCompat": false,
"forwardCompat": false
},
"Interface_ISummarizerEvents": {
"backCompat": false,
"forwardCompat": false
}
},
"entrypoint": "legacy"
}
}
3 changes: 0 additions & 3 deletions packages/runtime/container-runtime/src/index.ts
Original file line number Diff line number Diff line change
@@ -55,7 +55,6 @@ export {
ISummaryCancellationToken,
neverCancelledSummaryToken,
Summarizer,
SummarizerStopReason,
SummaryCollection,
EnqueueSummarizeResult,
IAckSummaryResult,
@@ -75,7 +74,6 @@ export {
ISubmitSummaryOptions,
ISerializedElection,
ISummarizeOptions,
ISummarizerEvents,
ISummarizerInternalsProvider,
ISummarizerRuntime,
ISummarizingWarning,
@@ -95,7 +93,6 @@ export {
SubmitSummaryFailureData,
SummaryStage,
IRetriableFailureError,
ISummarizeEventProps,
IdCompressorMode,
IDocumentSchema,
DocumentSchemaValueType,
3 changes: 0 additions & 3 deletions packages/runtime/container-runtime/src/summary/index.ts
Original file line number Diff line number Diff line change
@@ -44,12 +44,10 @@ export {
ISummarizeHeuristicData,
ISummarizer,
ISummarizeResults,
ISummarizerEvents,
ISummarizerInternalsProvider,
ISummarizerRuntime,
ISummaryCancellationToken,
SubmitSummaryResult,
SummarizerStopReason,
EnqueueSummarizeResult,
IAckSummaryResult,
IBaseSummarizeResult,
@@ -67,7 +65,6 @@ export {
SubmitSummaryFailureData,
SummaryStage,
IRetriableFailureError,
ISummarizeEventProps,
} from "./summarizerTypes.js";
export {
IAckedSummary,
65 changes: 6 additions & 59 deletions packages/runtime/container-runtime/src/summary/summarizerTypes.ts
Original file line number Diff line number Diff line change
@@ -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,
@@ -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.
@@ -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.
*/
@@ -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.
Original file line number Diff line number Diff line change
@@ -742,6 +742,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>>

/*
@@ -751,6 +752,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>>

/*
@@ -814,6 +816,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>>

/*
@@ -823,6 +826,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>>

/*
@@ -1426,6 +1430,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>>

/*
@@ -1435,6 +1440,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>>

/*