Skip to content

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

Merged
merged 32 commits into from
May 6, 2025

Conversation

scottn12
Copy link
Contributor

@scottn12 scottn12 commented Apr 22, 2025

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 from compatibilityMode -> compatibilityVersion in #24407).

Considerations

The API was added to LoadContainerRuntimeParams for two reasons:

  1. Allow us to give clear guidance to customers. For example, "Pass in minVersionForCollab X to ensure collaboration works with clients running runtime version X.
  2. It provides a simple path to requiring customers to pass in a 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:

const containerRuntime = await ContainerRuntime.loadRuntime({
    context,
    registryEntries,
    existing,
    provideEntryPoint,
    runtimeOptions: {
	enableGroupedBatching: false,
    },
    minVersionForCollab: "2.20.0", 
});

There are two noteworthy observations:

  1. minVersionForCollab is set to "2.20.0". This will ensure the default configurations for IContainerRuntimeOptionsInternal are set such that we will ensure that clients running version 2.20.0 or later of the runtime can collaborate.
  2. We also manually set enableGroupedBatching in the runtimeOptions param. Batching is normally enabled by default when minVersionForCollab is set to "2.0.0" or later, but any configurations manually set in runtimeOptions will override the default configurations.

Next Steps

  • Add "fail fast" mechanism that prevents explicity setting runtimeOptions in such a way that contradicts minVersionForCollab. For example, setting enableGroupedBatching to be true and minVersionForCollab 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#37810

Misc

AB#36088

@github-actions github-actions bot added area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct area: runtime Runtime related issues public api change Changes to a public API base: main PRs targeted against main branch labels Apr 22, 2025
@scottn12 scottn12 marked this pull request as ready for review April 22, 2025 19:47
@Copilot Copilot AI review requested due to automatic review settings April 22, 2025 19:47
@scottn12 scottn12 requested a review from a team as a code owner April 22, 2025 19:47
Copy link
Contributor

@Copilot Copilot AI left a 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> = {

@agarwal-navin
Copy link
Contributor

We also manually set enableGroupedBatching in the runtimeOptions param. Batching is normally enabled by default when minVersionForCollab is set to "2.0.0" or later, but any configurations manually set in runtimeOptions will override the default configurations.

What happens when an option explicitly passed via runtimeOptions conflicts with the options generated via minVersionForCollab such that it would break collab?
For instance, say minVersionForCollab is set to "1.4.0" in the above example and enableGroupedBatching is set to true. enableGroupedBatching was added in "2.0.0", so clients running 1.4.0 won't be able to collaborate rendering the minVersionForCollab useless. This can add to confusion to customers who were likely expecting collab to work.

Copy link
Contributor

@jason-ha jason-ha left a 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

jason-ha
jason-ha previously approved these changes May 6, 2025
Copy link
Contributor

@jason-ha jason-ha left a 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.

@jason-ha jason-ha dismissed their stale review May 6, 2025 16:57

per new suggestions with the SemanticVersion type change

Copy link
Contributor

github-actions bot commented May 6, 2025

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output


> fluid-framework-docs-site@0.0.0 ci:check-links /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test "npm run serve -- --no-open" 3000 check-links

1: starting server using command "npm run serve -- --no-open"
and when url "[ 'http://127.0.0.1:3000' ]" is responding with HTTP status code 200
running tests using command "npm run check-links"


> fluid-framework-docs-site@0.0.0 serve
> docusaurus serve --no-open

[SUCCESS] Serving "build" directory at: http://localhost:3000/

> fluid-framework-docs-site@0.0.0 check-links
> linkcheck http://localhost:3000 --skip-file skipped-urls.txt

Crawling...

Stats:
  195689 links
    1565 destination URLs
    1797 URLs ignored
       0 warnings
       0 errors


@scottn12 scottn12 enabled auto-merge (squash) May 6, 2025 19:15
@scottn12 scottn12 merged commit 67afa66 into microsoft:main May 6, 2025
41 checks passed
@scottn12 scottn12 deleted the defaultConfigsAPI branch May 6, 2025 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct area: runtime Runtime related issues base: main PRs targeted against main branch public api change Changes to a public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants