Repository navigation
air: Announce non-git code_source snapshots before packaging - #7618
Draft
artemy-kolesnikov wants to merge 2 commits into
Draft
artemy-kolesnikov wants to merge 2 commits into
artemy-kolesnikov wants to merge 2 commits into
Conversation
When code_source.root_path is not a git repository, `air run` packages every non-ignored file in the directory and used to print only "Uploading <name>.tar.gz...". It now says so before the staging spinner starts, with the number of files and their uncompressed size, or that it reuses the cached snapshot. At 64 MiB or more it suggests code_source.include_paths or a .gitignore. The notice is skipped with -o json, like "Submitting experiment:". The count and size cover only what the tarball holds (regular files and symlinks, per tarpack.Archives). Listed paths that tarpack skips, such as a nested git repository, which ls-files lists as one directory, are named in a warning; --dry-run no longer counts them. The swallowed error from the git repository probe is now logged at debug level. Co-authored-by: Isaac <no-reply@databricks.com>
Contributor
|
Commit: Regression test reportSelected 1 changed tests (limit=20, 0 not selected) 1 tests: regression=1 unreleased=0 coverage=0 failing=0
TestAccept/air/run-submit-non-git: branch ✅ | main (c4559ed) ❌ | latest (v1.26.0) ❌main (c4559ed): latest (v1.26.0): |
Collaborator
✅ Integration test reportCommit: e7059e1
Top 3 slowest tests (at least 2 minutes):
|
Co-authored-by: Isaac <no-reply@databricks.com>
This branch has not been deployed
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
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.
Title: air: Announce non-git code_source snapshots before packaging
Changes
When
code_source.root_pathis not a git repository,air runfalls back to a plain tarball of every non-ignored file in the directory. Onmainthe only output isUploading <name>.tar.gz..., so a user can ship logs, data or scratch files without noticing (a liverun.logwas shipped this way in a real workspace). In text mode,air runnow prints before the "Staging run artifacts…" spinner starts:No git repository at <root>; reusing the cached snapshot (N file(s), <size> uncompressed).instead, so no non-git submission is silent.Tip: limit the snapshot with code_source.include_paths or a .gitignore in the directory.The threshold issnapshotLargeSourceBytes, an arbitrary default that has not been measured and can be tuned. It applies to uncompressed source bytes, because that is whatinclude_pathsand.gitignorecontrol. The compressed tarball size stays telemetry-only.-o jsonthe notice is skipped, the same waySubmitting experiment:is. No JSON fields are added.tarpack.Archives, whichtarpack.Writenow uses too, so the two can't drift apart. If some listed paths are skipped, a warning names up to 5 of them, in every output mode:Warning: the snapshot leaves out 1 listed path(s) that are not regular files or symlinks, such as nested git repositories: lib/.--dry-run'swould include N file(s)uses the same count.isRepositorydiscards, is now logged at debug level (--debug).git_archive, andplain_tarinside a git repository) print nothing new.To print before the spinner,
uploadSnapshotis split in two.prepareSnapshotresolves the root path and plan, lists the files, names the tarball and checks whether it is already uploaded.preparedSnapshot.uploadpackages and uploads.submitWorkloadruns the first step, thenannounceSnapshot, before the staging errgroup. Packaging and upload still overlap server-side validation as before. The listing and oneStatrequest now run before validation starts rather than alongside it.Why
The Python AIR CLI already prints
Non-git directory detected, will create plain tarball without version caching(in the Python AIR CLI's submit path). The Go port never had this notice, and the non-git branch has existed without it since the first Go integration (#5847).Not addressed here:
fatal: not a git repositoryline in the report is still unexplained, and it does not come from the CLI. Every git subprocess inair runcaptures its stderr. The only git call against a non-gitroot_path(rev-parse --is-inside-work-tree) swallows the error, and running both v1.26.0 andmainin a non-git directory prints nofatal:. The line came from somewhere else, such as the reporter's shell or harness or remote output relayed by--watch, and this change does not claim to explain it.root_pathis listed byls-files --othersas a single directory entry.tarpackskips directories, so none of that repository's files ship. This change makes the loss visible (the warning above, and correct counts) but does not include the files. The same gap means a root whose only content is a nested repository gets an empty tarball past the air: refuse to upload an empty code_source snapshot #7570 empty-snapshot check.Tests
cmd/air/snapshot_notice_test.go:shippedSnapshotFilesagainst tarpack's rule (a directory is skipped, a symlink counts with 0 bytes).main.py,run.logand a nestedgit init'dlib/, run throughsubmitWorkload. The uploaded tarball holds exactlyproj/main.pyandproj/run.log; the notice says2 file(s) (19 B uncompressed); the warning nameslib/;countSnapshotFiles(dry-run) returns 2.submitWorkloadwithcmdio.NewTestIO, which forces an interactive stderr so the bubbletea spinner really renders. The warning and the notice come before the spinner's first escape sequence and beforeUploading. If the announce call is moved inside the staging closure, the test fails 3 runs out of 3.showProgress=false(JSON mode) prints no notice but still warns; a git repository gets no notice.acceptance/air/run-submit-non-git: a non-git submit with exact count and size (5 file(s) (64.3 KiB uncompressed)), an unchanged resubmit (reuse line), and-o json(no notice; clean envelope).go test ./cmd/air/... ./libs/tarpack/... -count=1passes. The wholeTestAccept/air/suite passes, including allrun-submit*fixtures; existing goldens are unchanged../task lint-q-go-root(0 issues),./task fmt-q-go,./task deadcodeand gofmt are clean.main.py,workload.yaml,run.logand a 64 KB file:--dry-runoutput is identical with the base and fixed builds (would include 4 file(s)).This pull request and its description were written by Isaac.