[fluid-runner] Expose createFluidRunnerContainerAndExecute and createFluidRunnerLogger#27182
[fluid-runner] Expose createFluidRunnerContainerAndExecute and createFluidRunnerLogger#27182kian-thompson wants to merge 4 commits into
Conversation
…FluidRunnerLogger Co-authored-by: Copilot <copilot@github.com>
|
Hi! Thank you for opening this PR. Want me to review it? Based on the diff (188 lines, 9 files), I've queued these reviewers:
How this works
|
There was a problem hiding this comment.
Pull request overview
This PR expands the @fluidframework/fluid-runner legacy/beta public API surface to support partner scenarios by exposing lower-level container execution and file-backed telemetry logger helpers.
Changes:
- Export
createFluidRunnerContainerAndExecuteto load a container from an ODSP snapshot and run caller-provided execution logic. - Export
createFluidRunnerLoggerto create a file-backed telemetry logger intended to be passed into the new execution helper. - Promote
IFileLoggerandITelemetryOptionsfrom@internalto@legacy @beta, and update internal usage/tests accordingly.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/tools/fluid-runner/src/test/loggerUtils.spec.ts | Updates tests to use createFluidRunnerLogger. |
| packages/tools/fluid-runner/src/test/exportFile.spec.ts | Updates tests to use createFluidRunnerContainerAndExecute. |
| packages/tools/fluid-runner/src/parseBundleAndExportFile.ts | Switches internal call sites to the newly exposed helper APIs. |
| packages/tools/fluid-runner/src/logger/loggerUtils.ts | Renames/exposes logger creation helper and updates types/TSDoc tags. |
| packages/tools/fluid-runner/src/logger/fileLogger.ts | Promotes IFileLogger/ITelemetryOptions to @legacy @beta. |
| packages/tools/fluid-runner/src/index.ts | Re-exports the new helper APIs from the package entrypoint. |
| packages/tools/fluid-runner/src/exportFile.ts | Renames/exposes the container execution helper and adjusts logger typing. |
| packages/tools/fluid-runner/api-report/fluid-runner.legacy.beta.api.md | Updates generated legacy/beta API report to include new exports. |
| .changeset/expose-fluid-runner-helpers.md | Adds a changeset for the new legacy/beta API surface. |
Comments suppressed due to low confidence (2)
packages/tools/fluid-runner/src/logger/loggerUtils.ts:44
- Now that this helper is part of the public legacy/beta surface, consider validating
filePathinsidecreateFluidRunnerLogger(e.g. fail if it already exists).JSONFileLoggerappends to the file during initialization, which can corrupt output if an existing file is passed, and consumers may not callgetTelemetryFileValidationErrorfirst.
const fileLogger =
options?.outputFormat === OutputFormat.CSV
? new CSVFileLogger(filePath, options?.eventsPerFlush, options?.defaultProps)
: new JSONFileLogger(filePath, options?.eventsPerFlush, options?.defaultProps);
packages/tools/fluid-runner/src/exportFile.ts:153
- When
disableNetworkFetchis true, this function overwritesglobal.fetchbut never restores the original implementation. This leaks a process-wide side effect to callers (and any subsequent container loads) and can cause unrelated code to start failing after this helper returns. Please restore the previousglobal.fetchin afinallyblock (or otherwise scope the override) so the override only applies during the container load/execution.
if (disableNetworkFetch) {
global.fetch = async () => {
throw new Error("Network fetch is not allowed");
};
}
Co-authored-by: Copilot <copilot@github.com>
jason-ha
left a comment
There was a problem hiding this comment.
Make sure to build a FF test/ branch build and confirm integration with Pages.
They are using /internal imports and mocking right now. While that isn't "supported" it will be a problem for us if those integration breaks.
|
Since this didn't make 2.103, please wait to build new test build until after #27476 merges in 2.110 and merged to this branch. |
…r helpers Keep the previous names available as deprecated @internal aliases so existing internal consumers continue to work: - createContainerAndExecute -> createFluidRunnerContainerAndExecute - createLogger -> createFluidRunnerLogger - ITelemetryOptions -> IFileLoggerTelemetryOptions The deprecated functions retain the original implementations. The new @legacy @beta exports are thin wrappers that take/return the public ITelemetryBaseLogger surface. Also fixes a few coupled bugs introduced by the rename (missing createChildLogger import, stale baseLogger reference, stale ITelemetryOptions type reference) and updates the changeset to document the back-compat aliases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🔗 No broken links found! ✅ Your attention to detail is admirable. linkcheck output |
Our partner team requested some additional methods from the
fluid-runnerpackage.