move more things into containers-shared for cf - #15512
Draft
emily-shen wants to merge 1 commit into
Draft
Conversation
|
@cloudflare/autoconfig
@cloudflare/build-output-utils
@cloudflare/codemods
@cloudflare/config
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-functions
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-plugin
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
Comment on lines
+808
to
+831
| async function runPreviewContainerOperation<T>( | ||
| options: { quiet: boolean }, | ||
| operation: () => Promise<T> | ||
| ): Promise<T> { | ||
| initContainersSharedContext({ | ||
| logger: options.quiet ? quietLogger : logger, | ||
| fetchResult, | ||
| }); | ||
|
|
||
| if (!options.quiet) { | ||
| return operation(); | ||
| } | ||
|
|
||
| // Building and applying containers prints progress to stdout, the same stream | ||
| // that carries the `--json` payload. Keep both logging surfaces quiet while | ||
| // stdout has to stay machine-readable. | ||
| const previousLogLevel = getLogLevel(); | ||
| setLogLevel("error"); | ||
| try { | ||
| return await operation(); | ||
| } finally { | ||
| setLogLevel(previousLogLevel); | ||
| initContainersSharedContext({ logger, fetchResult }); | ||
| } |
Contributor
There was a problem hiding this comment.
P1: Keep generated client debug output out of preview --json. verifyContainersScope() initializes OpenAPI.LOGGER to the normal Wrangler logger during preparation, but this later quiet path only changes the containers context and the CLI-helper log level. With WRANGLER_LOG=debug, ApplicationsService logs request and response details to stdout before the JSON result. The previous runWithLogLevel("warn") scope suppressed this.
Suggested change
| async function runPreviewContainerOperation<T>( | |
| options: { quiet: boolean }, | |
| operation: () => Promise<T> | |
| ): Promise<T> { | |
| initContainersSharedContext({ | |
| logger: options.quiet ? quietLogger : logger, | |
| fetchResult, | |
| }); | |
| if (!options.quiet) { | |
| return operation(); | |
| } | |
| // Building and applying containers prints progress to stdout, the same stream | |
| // that carries the `--json` payload. Keep both logging surfaces quiet while | |
| // stdout has to stay machine-readable. | |
| const previousLogLevel = getLogLevel(); | |
| setLogLevel("error"); | |
| try { | |
| return await operation(); | |
| } finally { | |
| setLogLevel(previousLogLevel); | |
| initContainersSharedContext({ logger, fetchResult }); | |
| } | |
| async function runPreviewContainerOperation<T>( | |
| options: { quiet: boolean }, | |
| operation: () => Promise<T> | |
| ): Promise<T> { | |
| const containersLogger = options.quiet ? quietLogger : logger; | |
| initContainersSharedContext({ | |
| logger: containersLogger, | |
| fetchResult, | |
| }); | |
| if (!options.quiet) { | |
| return operation(); | |
| } | |
| const { OpenAPI } = await import("@cloudflare/containers-shared"); | |
| const previousOpenApiLogger = OpenAPI.LOGGER; | |
| OpenAPI.LOGGER = { | |
| ...quietLogger, | |
| debugWithSanitization() {}, | |
| }; | |
| // Building and applying containers prints progress to stdout, the same stream | |
| // that carries the `--json` payload. Keep both logging surfaces quiet while | |
| // stdout has to stay machine-readable. | |
| const previousLogLevel = getLogLevel(); | |
| setLogLevel("error"); | |
| try { | |
| return await operation(); | |
| } finally { | |
| setLogLevel(previousLogLevel); | |
| OpenAPI.LOGGER = previousOpenApiLogger; | |
| initContainersSharedContext({ logger, fetchResult }); | |
| } | |
| } |
Contributor
|
I'm Bonk, and I've done a quick review of your PR. Moves container build, deployment, and preview logic into
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Also:
move container build earlier
A picture of a cute animal (not mandatory, but encouraged)