Skip to content

normalize pkey#8

Open
susanhooks wants to merge 3 commits into
mainfrom
sh/normalize-ib-pkey
Open

normalize pkey#8
susanhooks wants to merge 3 commits into
mainfrom
sh/normalize-ib-pkey

Conversation

@susanhooks
Copy link
Copy Markdown
Collaborator

@susanhooks susanhooks commented May 28, 2026

Description

Add normalization for pkey

Validation

  • Standard CI passes.
  • [] Kind integration passes, or this PR explains why it was not run.

The kind integration test is manual due to taking ~30 min to complete. When the PR is ready for review,
run Actions -> Kind Integration -> Run workflow against the copy-pr-bot generated
pull-request/<PR_NUMBER> branch. Use the default test_path for the full suite,
or narrow it only while debugging.

Passing Kind Integration run:

Checklist

  • I am familiar with the contributing guidelines in CONTRIBUTING.md.
  • Commits are signed off for DCO compliance.
  • New or existing tests cover these changes, or the PR explains why tests are not needed.
  • Documentation is updated for user-facing behavior changes.
  • Generated artifacts are updated when applicable, such as OpenAPI specs,
    docs screenshots, or Helm/rendered outputs.

Summary by CodeRabbit

  • New Features

    • InfiniBandPKey pkey values are now automatically normalized to a canonical lowercase 0x + 4 hex-digit format. Normalization occurs during field validation and model persistence to ensure consistent data representation.
  • Tests

    • Added comprehensive unit tests verifying pkey normalization behavior, including handling of whitespace, various hex digit cases, and invalid inputs.

Review Change Stack

@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 28, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 28, 2026

📝 Walkthrough

Walkthrough

This PR extends InfiniBandPKey with pkey normalization to a canonical lowercase 0x + 4-hex-digit format. It adds a regex-based normalize_pkey() static method and integrates normalization into model lifecycle methods (clean_fields, clean, save), plus comprehensive test coverage.

Changes

InfiniBandPKey pkey normalization

Layer / File(s) Summary
Normalization implementation
components/nautobot/nautobot-app-overlays/nautobot_app_overlays/models.py
Adds re import and implements normalize_pkey() static method with regex validation. Wires normalization into clean_fields(), clean(), and save() methods to ensure pkey is canonicalized before validation and persistence.
Normalization test coverage
components/nautobot/nautobot-app-overlays/nautobot_app_overlays/tests/test_models.py
Tests normalize_pkey() with valid (whitespace, varying casing/length) and invalid inputs, verifies model save() persists normalized values, and confirms full_clean() normalizes before validation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A pkey hops into the light,
Its digits trimmed, its format tight—
With regex charm and lowercase grace,
This InfiniBand finds its place.
Normalized, clean, and saved with care,
The perfect form now everywhere! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'normalize pkey' is vague and generic, lacking specificity about the context or component being modified. Consider using a more descriptive title such as 'Normalize InfiniBandPKey values to canonical format' or 'Add pkey normalization with validation' to better convey the change.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sh/normalize-ib-pkey

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@components/nautobot/nautobot-app-overlays/nautobot_app_overlays/models.py`:
- Around line 374-377: The save() method canonicalizes pkey but downstream
lookup _find_existing_pkey may be doing exact-match on raw input, causing
unique_together conflicts; update _find_existing_pkey to call
NautobotAppOverlay.normalize_pkey(input_pkey) (or otherwise canonicalize the
lookup key) before querying and ensure any create payload paths also run
normalize_pkey on incoming pkey values so lookups and inserts use the same
canonical form (reference save(), normalize_pkey(), and _find_existing_pkey()).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d5a83c7c-e022-4c88-bc98-5385e3b7e616

📥 Commits

Reviewing files that changed from the base of the PR and between a51cede and 95423ca.

📒 Files selected for processing (2)
  • components/nautobot/nautobot-app-overlays/nautobot_app_overlays/models.py
  • components/nautobot/nautobot-app-overlays/nautobot_app_overlays/tests/test_models.py

@susanhooks
Copy link
Copy Markdown
Collaborator Author

/ok to test a09ac73

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant