refactor(validator): move v1 schema from pkg/api/validator/v1 to pkg/validator/v1#1117
Merged
Merged
Conversation
…validator/v1
The validator-catalog wire schema lived under pkg/api/ but pkg/api/ is
documented in docs/integrator/public-api.md as Internal HTTP-handler
code ("Server-side REST handlers (server-internal; consumers use the
HTTP API, not the Go types)"). The v1 schema isn't HTTP-specific — it's
the YAML wire format the validator package consumes — so its location
mis-labels its actual stability tier and forces sibling-into-internal
imports from pkg/validator/catalog → pkg/api/validator/v1.
Move pkg/api/validator/v1/ → pkg/validator/v1/ via git mv (history
preserved on all 14 files). Package name unchanged (still `v1`); only
import path changes. Symmetric with the pkg/client/v1 facade location
established by #1111 — both are now pkg/<domain>/v1/.
External Go importers reaching into pkg/api/validator/v1 (despite the
Internal label in public-api.md) need to update:
- import v1 "github.com/NVIDIA/aicr/pkg/api/validator/v1"
+ import v1 "github.com/NVIDIA/aicr/pkg/validator/v1"
In-tree imports (13 files across pkg/cli, pkg/client/v1, pkg/validator,
validators/) updated mechanically.
No code changes; pure rename + import path update. Build, lint, and
tests clean on all affected packages.
Contributor
Coverage Report ✅
Coverage BadgeNo Go source files changed in this PR. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (265)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR repoints many imports from github.com/NVIDIA/aicr/pkg/api/validator/v1 to github.com/NVIDIA/aicr/pkg/validator/v1 across core validator packages (pkg/validator/validator.go, pkg/validator/phases.go), subsystems (pkg/validator/catalog, pkg/validator/job), client/CLI facades (pkg/client/v1/aicr.go, pkg/cli/validate.go), validator implementations and tests (validators/* and pkg/validator/* tests), and updates go.mod with multiple indirect dependency version bumps. The catalog re-exports and validators.Context exported field now resolve to types in the relocated package. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pkg/validator/catalog/catalog.go (1)
32-32:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix stale package reference in the re-export comment.
The comment still points to
pkg/api/validator/v1; it should referencepkg/validator/v1to match the current aliases.As per coding guidelines
Ensure code comments are accurate and helpful.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/validator/catalog/catalog.go` at line 32, The comment above the re-exports in catalog.go incorrectly references pkg/api/validator/v1; update that comment to reference the current package alias pkg/validator/v1 so it accurately documents the re-exported types (search for the comment line starting with "Re-exported types" in pkg/validator/catalog/catalog.go and replace the old package path with pkg/validator/v1).pkg/validator/phases.go (1)
19-23:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale re-export comments to the new package path.
These comments still reference
pkg/api/validator/v1, but the re-export source is nowpkg/validator/v1. Keeping the comment aligned avoids migration confusion.As per coding guidelines
Ensure code comments are accurate and helpful.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/validator/phases.go` around lines 19 - 23, Update the stale comment above the re-exports so it references the correct package path (pkg/validator/v1) instead of pkg/api/validator/v1; specifically adjust the comment lines that precede the re-exported type declaration "type Phase = v1.Phase" and the constants block starting with "const (" so the comments accurately describe the source of these re-exports.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@pkg/validator/catalog/catalog.go`:
- Line 32: The comment above the re-exports in catalog.go incorrectly references
pkg/api/validator/v1; update that comment to reference the current package alias
pkg/validator/v1 so it accurately documents the re-exported types (search for
the comment line starting with "Re-exported types" in
pkg/validator/catalog/catalog.go and replace the old package path with
pkg/validator/v1).
In `@pkg/validator/phases.go`:
- Around line 19-23: Update the stale comment above the re-exports so it
references the correct package path (pkg/validator/v1) instead of
pkg/api/validator/v1; specifically adjust the comment lines that precede the
re-exported type declaration "type Phase = v1.Phase" and the constants block
starting with "const (" so the comments accurately describe the source of these
re-exports.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: e526056a-b3c3-44b4-8b87-f8b4ef5294f5
📒 Files selected for processing (28)
pkg/cli/validate.gopkg/client/v1/aicr.gopkg/validator/catalog/catalog.gopkg/validator/catalog/catalog_test.gopkg/validator/job/deployer.gopkg/validator/job/deployer_test.gopkg/validator/phases.gopkg/validator/v1/README.mdpkg/validator/v1/affinity.gopkg/validator/v1/affinity_test.gopkg/validator/v1/catalog.gopkg/validator/v1/catalog_test.gopkg/validator/v1/conversion_test.gopkg/validator/v1/dependency_affinity.gopkg/validator/v1/dependency_affinity_test.gopkg/validator/v1/doc.gopkg/validator/v1/job_plan.gopkg/validator/v1/job_plan_internal.gopkg/validator/v1/job_plan_test.gopkg/validator/v1/validation_input.gopkg/validator/v1/validation_input_test.gopkg/validator/validator.gopkg/validator/validator_test.govalidators/conformance/robust_controller_check_test.govalidators/context.govalidators/deployment/expected_resources_test.govalidators/performance/inference_perf_constraint.govalidators/performance/inference_perf_test.go
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.
Summary
Move the validator-catalog wire schema from
pkg/api/validator/v1topkg/validator/v1. Symmetric with thepkg/client/v1facade location established by #1111 — both are nowpkg/<domain>/v1/.Motivation / Context
The validator-catalog schema lived under
pkg/api/butpkg/api/is documented indocs/integrator/public-api.mdas Internal HTTP-handler code:The
v1schema isn't HTTP-specific — it's the YAML wire format the validator package consumes (and that external CTRF / validator-catalog integrators may import). Burying it inside an Internal HTTP-handlers package mislabels its actual stability tier and forces sibling-into-internal imports frompkg/validator/catalog→pkg/api/validator/v1.After the move:
pkg/api/is consistently HTTP-handler-only (matches its documented purpose)pkg/validator)pkg/<domain>/v1/is consistent:pkg/client/v1,pkg/validator/v1Fixes: N/A
Related: #1076 (epic, now closed by #1113), #1111 (
pkg/client/v1move precedent)Type of Change
pkg/api/validator/v1explicitly; consider follow-up)Component(s) Affected
cmd/aicr,pkg/cli) — import path update inpkg/cli/validate.gocmd/aicrd,pkg/api,pkg/server) — schema moved out; pkg/api is now HTTP-onlypkg/validator) — schema relocated under this packageImplementation Notes
git mvon all 14 files (history preserved):Package name unchanged (still
v1); only the import path changes. 13 in-tree imports updated mechanically acrosspkg/cli,pkg/client/v1,pkg/validator/*, andvalidators/*.gofmtre-runs reordered import groups in those files.pkg/api/validator/directory removed (now empty).Behavior
No runtime behavior change. Pure rename + import path update.
Testing
Risk Assessment
sed.Rollout notes: External Go importers that reached into
pkg/api/validator/v1(despite its Internal tier label inpublic-api.md) must update:No symbol changes — a sed-style rewrite is sufficient. Release notes for the next minor should call this out alongside the
pkg/aicr→pkg/client/v1move from #1111.Checklist
go test -race ./pkg/validator/... ./pkg/cli/... ./pkg/api/... ./pkg/client/v1/... ./validators/...)make lint)pkg/api/validator/v1is not explicitly listed indocs/integrator/public-api.md's package matrix (onlypkg/apiis); no doc edit required by this PRgit commit -S)