-
Notifications
You must be signed in to change notification settings - Fork 539
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
refactor(container-runtime): Add IProtocolOptions and CompatibilityMode to container-runtime #23951
base: main
Are you sure you want to change the base?
Conversation
enableRuntimeIdCompressor, | ||
chunkSizeInBytes = defaultChunkSizeInBytes, | ||
enableGroupedBatching = true, |
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.
Something I noticed - we have enableGroupedBatching (and compression) on by default. Based on recent compat policy discussions, I believe this should be disabled by default, since it breaks compataiblity with an adjacent major public release (1.x).
@@ -33,3 +32,8 @@ export type { | |||
MemberChangedListener, | |||
Myself, | |||
} from "./types.js"; | |||
|
|||
// Re-export so other packages don't need to pull in container-runtime | |||
// TODO: Should we re-export? |
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.
I don't see a great reason to re-export the API in fluid-static, but I believe this would be a breaking change to a public API. Do we need to stage removing the API completely from fluid-static?
// @alpha | ||
export interface IProtocolOptions { | ||
readonly compressionOptions?: ICompressionRuntimeOptions; | ||
readonly enableGCSweep?: true | undefined; |
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.
gcOptions
is a tricky case, since it contains a compatability/protocol related option. Ultimately, I decided to just include that specific option (enableGCSweep
) in IProtocolOptions
and leave the rest of in IContainerRuntimeOptions
.
🔗 Found some broken links! 💔 Run a link check locally to find them. See linkcheck output
|
TODO
CompressionAlgorithms
Description
This PR adds
IProtocolOptions
andCompatibilityMode
(moved from fluid-static) to the container-runtime package.IProtocolOptions
The options currently in
IContainerRuntimeOptions
can be categorized into two groups. The first group are options that we provide to customers to customize their FluidFramework experience depending on their needs (i.e.summaryOptions
). The second group are options where their main function is to to maintain compliance with our cross-client compatibility policy. For example,enableGroupedBatching
, since batching is a feature we would ideally recommend to all customers but is not compatible with 1.x clients.IProtocolOptions
contains the options in the second group. This was done to add clarity for both customers and FF devs on which options are compat-related.In this PR's implementation, the container runtime accepts inputs from both
IContainerRuntimeOptions
andIProtocolOptions
. If a property is provided inIProtocolOptions
, it will overwrite the value inIContainerRuntimeOptions
. Eventually, we will remove any properties fromIContainerRuntimeOptions
that overlap withIProtocolOptions
.CompatibilityMode
CompatibilityMode
was added in this PR to lay the groundwork for easy upgrade paths for customers. Additional changes are required, but it will ultimately allow customers to specify the "minimum required version" to collaborate. It will ultimately accomplish the following:By giving customers a "switch" they can flip once they reach saturation on a new version of FF, they will have an easy and clear upgrade path.
Note: Since container-runtime is a depedency of fluid-static, we needed to move
CompatibilityMode
to the container-runtime package to make it accessible within container-runtime.Reviewer Guidance
CompatibilityMode
andcompatibilityModeRuntimeOptions
from fluid-static?Next Steps
IContainerRuntimeOptions
that overlap withIProtocolOptions
(and eventually remove).AB#31227