docs(playwright): correct the baked-browser contract comment - #23
Open
slayerjain wants to merge 2 commits into
Open
docs(playwright): correct the baked-browser contract comment#23slayerjain wants to merge 2 commits into
slayerjain wants to merge 2 commits into
Conversation
Two inaccuracies in the comment added with the bake, both of the kind that invite someone to "fix" the consumer back into a broken state. HOME is set to /root in this image, not unset. The path conclusion was right, the reasoning for it was not. More importantly the comment claimed a baked image "short-circuits BOTH the MinIO restore and `npx playwright install`". Only the restore is skipped. enterprise-ui deliberately runs the install unguarded, because Playwright resolves a browser by revision: after a package-lock bump the baked directory is non-empty but holds the wrong build, so a guard keyed on emptiness would skip the install as well and every lane would fail with "Executable doesn't exist at .../chromium-<rev>". Left as written, this comment is an argument for reinstating exactly that guard. Signed-off-by: slayerjain <shubham@keploy.io>
The slim- (arm64) job on this PR failed, and not because of anything in it — this branch only edits a comment in keploy-ci-playwright. The failure is in keploy-ci-slim: + curl -sSfL https://github.com/sigstore/cosign/releases/download/v2.5.2/cosign-linux-arm64 curl: (35) Recv failure: Connection reset by peer A single-shot download of a static release binary, over QEMU arm64 emulation, with no retry. The mc download one line above it had just succeeded, and the amd64 job passed — so this is a transient reset, and the only reason it was fatal is that nothing retried it. Every binary download in this repo had the same shape. The fix is the one keploy-ci-java already uses (--retry 3 --retry-delay 2), applied consistently and with --retry-all-errors, which matters here: plain --retry covers timeouts and transient HTTP status codes, but NOT a connection reset mid-transfer, which is exit 35 — precisely what failed. Without that flag the retry would not have helped. Eleven downloads across six images now retry: mc (x3), cosign, golangci-lint and its checksums, the Go tarball, the Docker apt key, the NodeSource setup script, and the MongoDB tools deb. The two piped into gpg and bash are included — curl retries before it emits anything, so the pipe does not defeat it, and a reset there is just as fatal. Nothing else changes: same URLs, same flags otherwise, same verification. A real outage still fails the build after five attempts rather than hanging. Signed-off-by: slayerjain <shubham@keploy.io>
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
1 participant
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.
Follow-up to #22. Comment-only — no build change.
Two inaccuracies, both of the kind that invite someone to "fix" the consumer back into a broken state:
HOMEis set to/root, not unset. The path conclusion was right, the stated reason was not (verified by running the published image).A baked image short-circuits the MinIO restore, not
npx playwright install. The comment claimed both. Only the restore is skipped.That second one matters.
enterprise-uiruns the install unguarded on purpose: Playwright resolves a browser by revision, so after apackage-lock.jsonbump the baked directory is non-empty but holds the wrong build. A guard keyed on "is the directory non-empty" would then skip the install too, and every lane would fail withExecutable doesn't exist at .../chromium-<rev>— a routine dependency bump turned into a repo-wide outage.As written, the comment reads as an argument for reinstating precisely that guard.