You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Install Rust with rustup and a rust-toolchain.toml - #174
Pin the rust version (1.99.1) and replace dtolnay/rust-toolchain with a local action that installs the pinned toolchain with the runner's own rustup and makes it the default. scripts/diskgraph reads the same file for the wheel settings and cache key.
Why
NVIDIA's enterprise Actions allowlist permits dtolnay/rust-toolchain only at its current SHA. Dependabot's bump (#18) makes every workflow that uses it fail at startup, including testing.yml, kumo-engine-pr.yml, playwright_ui.yml and the required kumo-core-gpu.yml, and every future bump would do the same. It's better to rely on native rustup than some external action.
Behavior changes
Tests and clippy move from 1.98.1 to 1.99.1, the stable release that PR and release wheels already build with. Wheels now stay on that pinned version instead of following the runner image.
Clippy 1.99 flags the #[must_use] that async-trait generates on DatasetRegistry and GraphStorage, so double_must_use is allowed there until the 1.100 bump.
Backend Docker images build DiskGraph with 1.99.1, because the toml travels with packages/diskgraph.
Developers building under packages/diskgraph get 1.99.1, which rustup 1.28.1 or later installs automatically.
One source for the version, shared by CI, wheels, Docker and developers.
.github/actions/rust-toolchain/action.yml
New composite action
Install the pinned toolchain with the runner's rustup, make it the default, add components, and export CARGO_HOME, CARGO_INCREMENTAL=0 and CARGO_TERM_COLOR=always.
Replaces dtolnay/rust-toolchain; the exports keep build settings and rust-cache keys as before.
Requested change: stop using dtolnay/rust-toolchain, which fails to start on #18, and pin the latest stable instead of 1.98.1.
Verification: actionlint, shellcheck, the workflow security policy, and the change-map checker pass. Rust CI and the GPU check pass, and the manual kumo-engine-wheel.yml run built all four platforms.
Out of scope: the 1.100 bump, which removes the double_must_use allows.
It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.
No actionable comments were generated in the recent review.
Walkthrough
The change pins the diskgraph Rust toolchain to 1.99.1 and adds a local composite action to install it. Wheel settings read the configured channel and include it in the cache key. Wheel and testing workflows use the local action. The diskgraph core traits allow the double_must_use lint.
Merge Risk: Minimal · up to 56f48
No actionable merge-blocking risk is established from the available evidence.
Pre-merge checks: 2 · 1
Failed checks: 1 inconclusive
Check name
Status
Explanation
Resolution
Change Map Design Review
The required PR conversation comment beginning <!-- kumo-pr-change-map --> is not available. The checkout contains only the repository guidance that requires this comment (`docs/development/review.m…
Provide the full conversation comment beginning <!-- kumo-pr-change-map --> or enable PR conversation retrieval, then reassess each proposed change against the repository diff and consumers.
Passed checks: 2 passed
Check name
Status
Explanation
Pr Size
The pull request changes 9 files. This does not exceed the failure threshold of 40 files.
Public Resource Access
The pull request adds no inaccessible external URL or external repository reference. It replaces the external dtolnay/rust-toolchain references with the local ./.github/actions/rust-toolchain acti…
Full details: Public Resource Access
Explanation
The pull request adds no inaccessible external URL or external repository reference. It replaces the external dtolnay/rust-toolchain references with the local ./.github/actions/rust-toolchain action. The new Rust toolchain source is the public Rust 1.99.1 channel, and its manifest and checksum were accessible anonymously from static.rust-lang.org with HTTP 200.
Full details: Change Map Design Review
Explanation
The required PR conversation comment beginning <!-- kumo-pr-change-map --> is not available. The checkout contains only the repository guidance that requires this comment (docs/development/review.md:7-13), while the supplied PR description provides only a link and summary. The code diff is inspectable, but it cannot establish whether the proposed change map documented the observed failure, necessity, focused verification, and stopping condition in that same comment.
Resolution
Provide the full conversation comment beginning <!-- kumo-pr-change-map --> or enable PR conversation retrieval, then reassess each proposed change against the repository diff and consumers.
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Reviewing files that changed from the base of the PR and between 5d17c1d and 56f4874.
Files selected for processing: 4
.github/workflows/_diskgraph-wheel.yml
.github/workflows/kumo-engine-pr.yml
.github/workflows/kumo-engine-wheel.yml
.github/workflows/testing.yml
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 9 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
Follow-up Comments
Run the shared backend wheel build from the staged source directory.
5-7: Minor · Functional Correctness · Quick win
Run the shared backend wheel build from the staged source directory.
The CPU and GPU builders invoke docker/build_diskgraph_wheel from /home/${USERNAME}. Its cargo update --manifest-path call does not use the staged rust-toolchain.toml, so both backend paths perform a Cargo operation with the Docker-installed stable toolchain instead of 1.98.1.
Change the shared script so its Cargo and wheel-build operations run from source_dir.
Suggested fix
if [ ! -f "${source_dir}/pyproject.toml" ]; then
echo "No staged DiskGraph pyproject.toml found at ${source_dir}" >&2
exit 1
fi
+ cd "$source_dir"
ensure_cargo
Reviewing files that changed from the base of the PR and between 70637d3 and e8c09ca.
Files selected for processing: 7
.github/actions/rust-toolchain/action.yml
.github/workflows/_diskgraph-wheel.yml
.github/workflows/kumo-engine-pr.yml
.github/workflows/kumo-engine-wheel.yml
.github/workflows/testing.yml
packages/diskgraph/rust-toolchain.toml
scripts/diskgraph
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.
pyo3 0.28's #[pyclass] expansion trips clippy 1.99's
clone_on_copy, which #174 allows in diskgraph-pybind. pyo3
0.29.3 fixes it, so drop the allow. arrow-pyarrow needs pyo3
0.29 from 59.1 and pyo3-log 0.13.5 accepts it, so arrow,
parquet and pyo3-log move together.
Follow their API changes: FixedSizeBinaryArray's Vec
conversions are now TryFrom (tests only),
MutableArrayData::extend is deprecated for try_extend, and
pyo3-build-config fields are read through getters. Regenerate
Cargo.lock, the artifact manifest, the notice inventory and
THIRD-PARTY.txt. Drop build_diskgraph_wheel's cargo update of
time: the staged source's Cargo.lock already pins 0.3.47.
Parquet files DiskGraph writes now mark float columns with
IEEE 754 total order, so pyarrow no longer prunes row groups
on their min/max. The upgrade also clears seven RUSTSEC
advisories in pyo3, zstd and thrift.
The reason will be displayed to describe this comment to others. Learn more.
Nitpick comments: 1
.github/workflows/testing.yml: 1
370-370: Trivial · Security & Privacy · Quick win
Use the $/ self-repository reference for the local Rust toolchain action.
./.github/actions/rust-toolchain resolves from the job workspace. $/.github/actions/rust-toolchain resolves from this repository at the running commit and does not need a checkout. The zizmor self-repository audit flags ./ references to in-repo actions and recommends $/.
The current checkout order means both forms load the same action file, so this change does not alter the toolchain that gets installed. Apply the same edit to the other new rust-toolchain references in _diskgraph-wheel.yml, kumo-engine-pr.yml, and kumo-engine-wheel.yml.
Reviewing files that changed from the base of the PR and between 41a9c5d and 9fb7ae5.
Files selected for processing: 1
.github/workflows/testing.yml
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
The enterprise allowlist permits dtolnay/rust-toolchain only at
its current SHA, so Dependabot's bump (#18) stops every workflow
that uses it from starting. Pin Rust in
packages/diskgraph/rust-toolchain.toml and install it with the
runner's rustup through a local action, which keeps exporting
CARGO_HOME, CARGO_INCREMENTAL=0 and CARGO_TERM_COLOR=always.
Pin 1.99.1, the stable release wheel builds already use, instead
of the 1.98.1 that tests and clippy used. Its clippy flags the
GraphStorage; allow it there. Backend images built from this
source follow the same pin.
## Summary
Enable NVIDIA's federated GCP authentication in the existing
GCP-containing general shards and GCS integration workflow. Remove
Google login from Snowflake, whose selected tests do not need GCP and
whose workflow is excluded by the provider policy.
GCS credential setup runs after pytest skip selection and before
resource fixtures, including session workers. Explicit ADC takes
precedence; BigQuery workload mode requires federated ADC. Existing test
shards, coverage commands, and check names are preserved. GCS runs for
every PR and exposes failures; general tests skip only known
documentation changes. Credential lifetime/type cleanup follows in #225.
## Change map
[Change
map](#224 (comment))
## Validation
Signed head `386c680b` includes main's merged #174 Rust action fix.
- 58 focused local tests pass; changed-file flake8/isort and diff checks
pass. General/GCS actionlint passes; Snowflake retains ten pre-existing
missing-step references.
- [GPU CI
passes](https://github.com/NVIDIA/kumo-engine/actions/runs/42147408907/job/127662279622).
- [Testing and Coverage
passes](https://github.com/NVIDIA/kumo-engine/actions/runs/42147407662),
including every pytest shard, package suites, native checks, and
aggregate coverage.
- [All three GCP integration jobs
pass](https://github.com/NVIDIA/kumo-engine/actions/runs/42147407538):
10 tests across GCS, BigQuery + S3, and BigQuery ingestion.
-
[Connector/config](https://github.com/NVIDIA/kumo-engine/actions/runs/42147407662/job/127662203622):
1915 passed, 41 skipped, 1 xfailed. Both restricted BigQuery permission
tests executed and passed.
- Real-source Snowflake collection with invalid encoded GCP credentials
succeeds: 104 filesystem tests skip without credential resolution;
connector/REST collection includes 1513 nodes.
The [Snowflake connector
job](https://github.com/NVIDIA/kumo-engine/actions/runs/42147407528/job/127662134899)
still fails one assertion because metadata includes static shared
`HM_SAMPLED.TEST_TABLE`. This identical failure predates these PRs on
[main](https://github.com/NVIDIA/kumo-engine/actions/runs/41911685209/job/126860619478).
Its creation/cleanup ownership has not been established; leftover state
and permission drift remain hypotheses. The assertion is retained
pending a supported fixture/contract fix. Existing mixed-shard
continue-on-error behavior and broader provider isolation remain outside
scope.
## Summary
Google integration tests require explicit local ADC files instead of
retrieving keys from AWS, decoding environment keys, or discovering
home-directory credentials. Typed BigQuery identities select restricted
key files or impersonation targets; workload mode requires federated
ADC.
Credential setup follows test skip selection and precedes resource
fixtures. The base identity survives worker teardown, each test restores
credential environment and GCS client cache, and input files remain
unchanged. Mixed local invocations that already started DiskGraph
workers fail with instructions to use a separate invocation. Depends on
#224; production connector authentication semantics and existing check
names are preserved.
## Change map
[Change
map](#225 (comment))
## Validation
Signed head `9920279c` includes main's merged #174 Rust action fix
through #224.
- 84 combined local credential/workflow tests pass; changed-file
flake8/isort and diff checks pass. Documentation checker: 257 files
pass.
- [GPU CI
passes](https://github.com/NVIDIA/kumo-engine/actions/runs/42147417204/job/127662289614).
- [Testing and Coverage
passes](https://github.com/NVIDIA/kumo-engine/actions/runs/42147415744),
including every pytest shard, package suites, native checks, and
aggregate coverage.
- [All three GCP integration jobs
pass](https://github.com/NVIDIA/kumo-engine/actions/runs/42147415537):
10 tests across GCS, BigQuery + S3, and BigQuery ingestion.
-
[Connector/config](https://github.com/NVIDIA/kumo-engine/actions/runs/42147415744/job/127662257242):
1915 passed, 41 skipped, 1 xfailed. Both restricted BigQuery permission
tests executed and passed.
- Real Snowflake filesystem invocation with invalid encoded GCP
credentials: 104 tests skip without credential resolution.
The [Snowflake connector
job](https://github.com/NVIDIA/kumo-engine/actions/runs/42147415442/job/127662161433)
still fails one assertion because metadata includes static shared
`HM_SAMPLED.TEST_TABLE`. This identical failure predates these PRs on
[main](https://github.com/NVIDIA/kumo-engine/actions/runs/41911685209/job/126860619478).
Its creation/cleanup ownership has not been established; leftover state
and permission drift remain hypotheses. The assertion is retained
pending a supported fixture/contract fix.
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
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.
What
Pin the rust version (1.99.1) and replace
dtolnay/rust-toolchainwith a local action that installs the pinned toolchain with the runner's own rustup and makes it the default.scripts/diskgraphreads the same file for the wheel settings and cache key.Why
NVIDIA's enterprise Actions allowlist permits
dtolnay/rust-toolchainonly at its current SHA. Dependabot's bump (#18) makes every workflow that uses it fail at startup, includingtesting.yml,kumo-engine-pr.yml,playwright_ui.ymland the requiredkumo-core-gpu.yml, and every future bump would do the same. It's better to rely on native rustup than some external action.Behavior changes
#[must_use]that async-trait generates onDatasetRegistryandGraphStorage, sodouble_must_useis allowed there until the 1.100 bump.packages/diskgraph.packages/diskgraphget 1.99.1, which rustup 1.28.1 or later installs automatically.Change map and validation