-
Notifications
You must be signed in to change notification settings - Fork 550
container-runtime: Add API for default configuration #24422
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
Conversation
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.
Pull Request Overview
This PR introduces a new API property, minVersionForCollab, to drive default configuration logic for the container runtime while renaming the legacy compatibilityVersion property. Key changes include renaming properties and functions, updating tests and docs to use minVersionForCollab, and adding relevant type exports.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
packages/runtime/container-runtime/src/test/containerRuntime.spec.ts | Updated test descriptions and usage to reference minVersionForCollab instead of compatibilityVersion |
packages/runtime/container-runtime/src/test/compatUtils.spec.ts | Updated test cases to use the new minVersionForCollab naming in place of compatibilityVersion |
packages/runtime/container-runtime/src/index.ts | Exported SemanticVersion from compatUtils for consistency |
packages/runtime/container-runtime/src/containerRuntime.ts | Renamed constants and default configuration functions from compatibilityVersion to minVersionForCollab and updated error messages accordingly |
packages/runtime/container-runtime/src/compatUtils.ts | Adjusted function names and constants to reference minVersionForCollab in generating default configurations |
packages/runtime/container-runtime/api-report/container-runtime.legacy.alpha.api.md | Added the new minVersionForCollab property to the API documentation |
packages/framework/fluid-static/src/rootDataObject.ts | Updated to pass minVersionForCollab in the container runtime instantiation and added a mapping from CompatibilityMode |
packages/framework/… | Updated compatibility configuration and aqueduct factory API report to expose the new minVersionForCollab |
Comments suppressed due to low confidence (3)
packages/runtime/container-runtime/src/containerRuntime.ts:155
- [nitpick] Consider renaming 'defaultminVersionForCollab' to 'defaultMinVersionForCollab' and 'getminVersionForCollabDefaults' to 'getMinVersionForCollabDefaults' to improve readability and maintain standard camelCase conventions.
import { defaultminVersionForCollab, getminVersionForCollabDefaults, isValidCompatVersion, type SemanticVersion, } from "./compatUtils.js";
packages/runtime/container-runtime/src/compatUtils.ts:41
- [nitpick] Rename 'defaultminVersionForCollab' to 'defaultMinVersionForCollab' for consistency with standard camelCase naming, which will make the code easier to read alongside other similarly named entities.
export const defaultminVersionForCollab = "2.0.0-defaults" as const;
packages/framework/fluid-static/src/rootDataObject.ts:45
- [nitpick] Consider renaming 'compatModeTominVersionForCollab' to 'compatModeToMinVersionForCollab' to clearly separate words and improve readability.
const compatModeTominVersionForCollab: Record<CompatibilityMode, SemanticVersion> = {
packages/runtime/container-runtime/src/test/containerRuntime.spec.ts
Outdated
Show resolved
Hide resolved
packages/framework/aqueduct/api-report/aqueduct.legacy.alpha.api.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Joshua Smithrud <54606601+Josmithr@users.noreply.github.com>
Co-authored-by: Joshua Smithrud <54606601+Josmithr@users.noreply.github.com>
What happens when an option explicitly passed via |
packages/framework/aqueduct/src/container-runtime-factories/baseContainerRuntimeFactory.ts
Show resolved
Hide resolved
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.
Looks good - few small notes/requests
packages/runtime/container-runtime/src/test/containerRuntime.spec.ts
Outdated
Show resolved
Hide resolved
packages/runtime/container-runtime/src/test/containerRuntime.spec.ts
Outdated
Show resolved
Hide resolved
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'm approving with note about an incomplete comment. So, feel free to merge when anyone else gives that final change approval.
packages/runtime/container-runtime/src/test/containerRuntime.spec.ts
Outdated
Show resolved
Hide resolved
packages/runtime/container-runtime/src/test/compatUtils.spec.ts
Outdated
Show resolved
Hide resolved
packages/runtime/container-runtime/src/test/containerRuntime.spec.ts
Outdated
Show resolved
Hide resolved
per new suggestions with the SemanticVersion type change
packages/framework/fluid-framework/api-report/fluid-framework.alpha.api.md
Outdated
Show resolved
Hide resolved
packages/runtime/container-runtime/src/test/compatUtils.spec.ts
Outdated
Show resolved
Hide resolved
🔗 No broken links found! ✅ Your attention to detail is admirable. linkcheck output
|
Description
This PR adds an API that allows customers to set leverage the default configuration logic added in #24379. The API is an optional property added to the
LoadContainerRuntimeParams
interface, which is used when instantiating a container runtime.This PR also renames the API from
compatibilityVersion
->minVersionForCollab
(it was changed fromcompatibilityMode
->compatibilityVersion
in #24407).Considerations
The API was added to
LoadContainerRuntimeParams
for two reasons:minVersionForCollab
X to ensure collaboration works with clients running runtime version X.minVersionForCollab
in the future (by making the param required).One alternative is to expose a helper function that will generate the
runtimeOptions
object that is used when instantiating a container runtime. However, this may complicate the guidance given to customers and does not provide a clear path to making this a required step in the future.Example
A customer may use the API like so:
There are two noteworthy observations:
minVersionForCollab
is set to"2.20.0"
. This will ensure the default configurations forIContainerRuntimeOptionsInternal
are set such that we will ensure that clients running version 2.20.0 or later of the runtime can collaborate.enableGroupedBatching
in theruntimeOptions
param. Batching is normally enabled by default whenminVersionForCollab
is set to"2.0.0"
or later, but any configurations manually set inruntimeOptions
will override the default configurations.Next Steps
runtimeOptions
in such a way that contradictsminVersionForCollab
. For example, settingenableGroupedBatching
to betrue
andminVersionForCollab
to"1.4.0"
. This should not be allowed since clients running v1.4.0 of the runtime will not be able to collaborate if batching is enabled. See AB#37810Misc
AB#36088