-
Notifications
You must be signed in to change notification settings - Fork 558
ConfigMap related cleanup and refactors #25868
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
base: main
Are you sure you want to change the base?
Changes from 5 commits
456127f
0b95e52
969ab28
fa7c86e
cc18602
3e6468d
f202424
99dc2b9
eab0505
be76340
7b7447e
92692c4
d7e33be
b5a7ab0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,7 @@ import { | |
| import { | ||
| configValueToMinVersionForCollab, | ||
| getConfigsForMinVersionForCollab, | ||
| getValidationForRuntimeOptions, | ||
| validateConfigMapOverrides, | ||
| type ConfigMap, | ||
| type ConfigValidationMap, | ||
| } from "@fluidframework/runtime-utils/internal"; | ||
|
|
@@ -101,60 +101,61 @@ export type RuntimeOptionKeysThatRequireExplicitSchemaControl = keyof Omit< | |
| * default value for `enableGroupedBatching` will be true because clients running 2.0 or later will be able to understand the format changes associated | ||
| * with the batching feature. | ||
| */ | ||
| const runtimeOptionsAffectingDocSchemaConfigMap = { | ||
| enableGroupedBatching: { | ||
| "1.0.0": false, | ||
| "2.0.0-defaults": true, | ||
| }, | ||
| compressionOptions: { | ||
| "1.0.0": disabledCompressionConfig, | ||
| "2.0.0-defaults": enabledCompressionConfig, | ||
| }, | ||
| enableRuntimeIdCompressor: { | ||
| // For IdCompressorMode, `undefined` represents a logical state (off). | ||
| // However, to satisfy the Required<> constraint while | ||
| // `exactOptionalPropertyTypes` is `false` (TODO: AB#8215), we need | ||
| // to have it defined, so we trick the type checker here. | ||
| "1.0.0": undefined, | ||
| // We do not yet want to enable idCompressor by default since it will | ||
| // increase bundle sizes, and not all customers will benefit from it. | ||
| // Therefore, we will require customers to explicitly enable it. We | ||
| // are keeping it as a DocSchema affecting option for now as this may | ||
| // change in the future. | ||
| }, | ||
| explicitSchemaControl: { | ||
| "1.0.0": false, | ||
| // This option's intention is to prevent 1.x clients from joining sessions | ||
| // when enabled. This is set to true when the minVersionForCollab is set | ||
| // to >=2.0.0 (explicitly). This is different than other 2.0 defaults | ||
| // because it was not enabled by default prior to the implementation of | ||
| // `minVersionForCollab`. | ||
| // `defaultMinVersionForCollab` is set to "2.0.0-defaults" which "2.0.0" | ||
| // does not satisfy to avoiding enabling this option by default as of | ||
| // `minVersionForCollab` introduction, which could be unexpected. | ||
| // Only enable as a default when `minVersionForCollab` is specified at | ||
| // 2.0.0+. | ||
| "2.0.0": true, | ||
| }, | ||
| flushMode: { | ||
| // Note: 1.x clients are compatible with TurnBased flushing, but here we elect to remain on Immediate flush mode | ||
| // as a work-around for inability to send batches larger than 1Mb. Immediate flushing keeps batches smaller as | ||
| // fewer messages will be included per flush. | ||
| "1.0.0": FlushMode.Immediate, | ||
| "2.0.0-defaults": FlushMode.TurnBased, | ||
| }, | ||
| gcOptions: { | ||
| "1.0.0": {}, | ||
| // Although sweep is supported in 2.x, it is disabled by default until minVersionForCollab>=3.0.0 to be extra safe. | ||
| "3.0.0": { enableGCSweep: true }, | ||
| }, | ||
| createBlobPayloadPending: { | ||
| // This feature is new and disabled by default. In the future we will enable it by default, but we have not | ||
| // closed on the version where that will happen yet. Probably a .10 release since blob functionality is not | ||
| // exposed on the `@public` API surface. | ||
| "1.0.0": undefined, | ||
| }, | ||
| } as const satisfies ConfigMap<RuntimeOptionsAffectingDocSchema>; | ||
| const runtimeOptionsAffectingDocSchemaConfigMap: ConfigMap<RuntimeOptionsAffectingDocSchema> = | ||
| { | ||
| enableGroupedBatching: { | ||
| "1.0.0": false, | ||
| "2.0.0-defaults": true, | ||
| }, | ||
| compressionOptions: { | ||
| "1.0.0": disabledCompressionConfig, | ||
| "2.0.0-defaults": enabledCompressionConfig, | ||
| }, | ||
| enableRuntimeIdCompressor: { | ||
| // For IdCompressorMode, `undefined` represents a logical state (off). | ||
| // However, to satisfy the Required<> constraint while | ||
| // `exactOptionalPropertyTypes` is `false` (TODO: AB#8215), we need | ||
| // to have it defined, so we trick the type checker here. | ||
| "1.0.0": undefined, | ||
| // We do not yet want to enable idCompressor by default since it will | ||
| // increase bundle sizes, and not all customers will benefit from it. | ||
| // Therefore, we will require customers to explicitly enable it. We | ||
| // are keeping it as a DocSchema affecting option for now as this may | ||
| // change in the future. | ||
| }, | ||
| explicitSchemaControl: { | ||
| "1.0.0": false, | ||
| // This option's intention is to prevent 1.x clients from joining sessions | ||
| // when enabled. This is set to true when the minVersionForCollab is set | ||
| // to >=2.0.0 (explicitly). This is different than other 2.0 defaults | ||
| // because it was not enabled by default prior to the implementation of | ||
| // `minVersionForCollab`. | ||
| // `defaultMinVersionForCollab` is set to "2.0.0-defaults" which "2.0.0" | ||
| // does not satisfy to avoiding enabling this option by default as of | ||
| // `minVersionForCollab` introduction, which could be unexpected. | ||
| // Only enable as a default when `minVersionForCollab` is specified at | ||
| // 2.0.0+. | ||
| "2.0.0": true, | ||
| }, | ||
| flushMode: { | ||
| // Note: 1.x clients are compatible with TurnBased flushing, but here we elect to remain on Immediate flush mode | ||
| // as a work-around for inability to send batches larger than 1Mb. Immediate flushing keeps batches smaller as | ||
| // fewer messages will be included per flush. | ||
| "1.0.0": FlushMode.Immediate, | ||
| "2.0.0-defaults": FlushMode.TurnBased, | ||
| }, | ||
| gcOptions: { | ||
| "1.0.0": {}, | ||
| // Although sweep is supported in 2.x, it is disabled by default until minVersionForCollab>=3.0.0 to be extra safe. | ||
| "3.0.0": { enableGCSweep: true }, | ||
| }, | ||
| createBlobPayloadPending: { | ||
| // This feature is new and disabled by default. In the future we will enable it by default, but we have not | ||
| // closed on the version where that will happen yet. Probably a .10 release since blob functionality is not | ||
| // exposed on the `@public` API surface. | ||
| "1.0.0": undefined, | ||
| }, | ||
| }; | ||
|
|
||
| /** | ||
| * Keys of {@link ContainerRuntimeOptionsInternal} that require explicitSchemaControl to be enabled. | ||
|
|
@@ -169,37 +170,39 @@ export const runtimeOptionKeysThatRequireExplicitSchemaControl = ( | |
| ); | ||
| }) as RuntimeOptionKeysThatRequireExplicitSchemaControl[]; | ||
|
|
||
| const runtimeOptionsAffectingDocSchemaConfigValidationMap = { | ||
| enableGroupedBatching: configValueToMinVersionForCollab([ | ||
| [false, "1.0.0"], | ||
| [true, "2.0.0-defaults"], | ||
| ]), | ||
| compressionOptions: configValueToMinVersionForCollab([ | ||
| [{ ...disabledCompressionConfig }, "1.0.0"], | ||
| [{ ...enabledCompressionConfig }, "2.0.0-defaults"], | ||
| ]), | ||
| enableRuntimeIdCompressor: configValueToMinVersionForCollab([ | ||
| [undefined, "1.0.0"], | ||
| ["on", "2.0.0-defaults"], | ||
| ["delayed", "2.0.0-defaults"], | ||
| ]), | ||
| explicitSchemaControl: configValueToMinVersionForCollab([ | ||
| [false, "1.0.0"], | ||
| [true, "2.0.0-defaults"], | ||
| ]), | ||
| flushMode: configValueToMinVersionForCollab([ | ||
| [FlushMode.Immediate, "1.0.0"], | ||
| [FlushMode.TurnBased, "2.0.0-defaults"], | ||
| ]), | ||
| gcOptions: configValueToMinVersionForCollab([ | ||
| [{ enableGCSweep: undefined }, "1.0.0"], | ||
| [{ enableGCSweep: true }, "2.0.0-defaults"], | ||
| ]), | ||
| createBlobPayloadPending: configValueToMinVersionForCollab([ | ||
| [undefined, "1.0.0"], | ||
| [true, "2.40.0"], | ||
| ]), | ||
| } as const satisfies ConfigValidationMap<RuntimeOptionsAffectingDocSchema>; | ||
| // A lot of the information in this seems redundant with whats defined above. Might be nice to combine them somehow. | ||
| const runtimeOptionsAffectingDocSchemaConfigValidationMap: ConfigValidationMap<RuntimeOptionsAffectingDocSchema> = | ||
| { | ||
| enableGroupedBatching: configValueToMinVersionForCollab([ | ||
| [false, "1.0.0"], | ||
| [true, "2.0.0-defaults"], | ||
| ]), | ||
| compressionOptions: configValueToMinVersionForCollab([ | ||
| [{ ...disabledCompressionConfig }, "1.0.0"], | ||
| [{ ...enabledCompressionConfig }, "2.0.0-defaults"], | ||
| ]), | ||
| enableRuntimeIdCompressor: configValueToMinVersionForCollab([ | ||
| [undefined, "1.0.0"], | ||
| ["on", "2.0.0-defaults"], | ||
| ["delayed", "2.0.0-defaults"], | ||
| ]), | ||
| explicitSchemaControl: configValueToMinVersionForCollab([ | ||
| [false, "1.0.0"], | ||
| [true, "2.0.0-defaults"], | ||
| ]), | ||
| flushMode: configValueToMinVersionForCollab([ | ||
| [FlushMode.Immediate, "1.0.0"], | ||
| [FlushMode.TurnBased, "2.0.0-defaults"], | ||
| ]), | ||
| gcOptions: configValueToMinVersionForCollab([ | ||
| [{ enableGCSweep: undefined }, "1.0.0"], | ||
| [{ enableGCSweep: true }, "2.0.0-defaults"], | ||
| ]), | ||
| createBlobPayloadPending: configValueToMinVersionForCollab([ | ||
| [undefined, "1.0.0"], | ||
| [true, "2.40.0"], | ||
| ]), | ||
| }; | ||
|
|
||
| /** | ||
| * Returns the default RuntimeOptionsAffectingDocSchema configuration for a given minVersionForCollab. | ||
|
|
@@ -210,9 +213,7 @@ export function getMinVersionForCollabDefaults( | |
| return getConfigsForMinVersionForCollab( | ||
| minVersionForCollab, | ||
| runtimeOptionsAffectingDocSchemaConfigMap, | ||
| // This is a bad cast away from Partial that getConfigsForCompatMode provides. | ||
| // ConfigMap should be restructured to provide RuntimeOptionsAffectingDocSchema guarantee. | ||
| ) as RuntimeOptionsAffectingDocSchema; | ||
|
Comment on lines
-213
to
-215
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removing this cast is possible now because getConfigsForMinVersionForCollab can no longer return undefined for entries, which if returned here would have been unsupported in RuntimeOptionsAffectingDocSchema. This code is now much more type safe since we no longer have a cast that could hide typing errors, and we no longer have the risk of undefined values slipping through as the FlushMode if the config changes or the collab version is lower than the lowest entry for them in the config map. |
||
| ); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -224,9 +225,9 @@ export function validateRuntimeOptions( | |
| minVersionForCollab: MinimumVersionForCollab, | ||
| runtimeOptions: Partial<ContainerRuntimeOptionsInternal>, | ||
| ): void { | ||
| getValidationForRuntimeOptions<RuntimeOptionsAffectingDocSchema>( | ||
| validateConfigMapOverrides<RuntimeOptionsAffectingDocSchema>( | ||
| minVersionForCollab, | ||
| runtimeOptions as Partial<RuntimeOptionsAffectingDocSchema>, | ||
| runtimeOptions, | ||
| runtimeOptionsAffectingDocSchemaConfigValidationMap, | ||
| ); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using satisfies instead of the variable type here doesn't help anything, so I have changed it to the simpler more conventional pattern. This also likely improves error messages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for runtimeOptionsAffectingDocSchemaConfigMap above