Skip to content

fix: warn-and-skip on duplicate repo keys instead of crashing#81

Merged
marcinpsk merged 1 commit into
mainfrom
fix/export
May 27, 2026
Merged

fix: warn-and-skip on duplicate repo keys instead of crashing#81
marcinpsk merged 1 commit into
mainfrom
fix/export

Conversation

@marcinpsk
Copy link
Copy Markdown
Owner

@marcinpsk marcinpsk commented May 27, 2026

The upstream DTL repo legitimately contains files with the same model name but different part numbers (e.g. HPE 38L7559.yaml and 834167-001.yaml both have model 'HPE Drive LTO-7 Ultrium 7-SCSI'). Previously the loaders raised ValueError which crashed export-diff.

Change all three loaders (_load_repo_device_types, _load_repo_module_types, _load_repo_rack_types) to log a verbose warning and keep the first-seen entry, matching the import path's behaviour when encountering same-name records.

Summary by CodeRabbit

  • Bug Fixes
    • Configuration files with duplicate entries are now handled gracefully with detailed logging instead of causing the application to fail. The first entry is preserved.

Review Change Stack

The upstream DTL repo legitimately contains files with the same
model name but different part numbers (e.g. HPE 38L7559.yaml and
834167-001.yaml both have model 'HPE Drive LTO-7 Ultrium 7-SCSI').
Previously the loaders raised ValueError which crashed export-diff.

Change all three loaders (_load_repo_device_types,
_load_repo_module_types, _load_repo_rack_types) to log a verbose
warning and keep the first-seen entry, matching the import path's
behaviour when encountering same-name records.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3b1f6787-3d94-45b9-8704-d841b3007097

📥 Commits

Reviewing files that changed from the base of the PR and between 1572c95 and e7ecbf7.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • core/export.py

Walkthrough

This PR modifies core/export.py to change duplicate key handling across three YAML loaders. Device-type, module-type, and rack-type repo loaders now emit verbose logs reporting duplicate key file locations and retain the first-seen entry instead of raising exceptions.

Changes

Duplicate Key Handling

Layer / File(s) Summary
Device-type, module-type, and rack-type duplicate key logging
core/export.py
Device-type loader (lines 516–520), module-type loader (lines 542–546), and rack-type loader (lines 568–572) replace ValueError exceptions with logging that reports both the earlier and duplicate file paths, then skip the later duplicate while keeping the first-seen YAML entry.

🎯 2 (Simple) | ⏱️ ~8 minutes

🐰 Three loaders once threw and crashed,
Now they log and gently pass,
First one stays, the rest are hashed,
Duplicates become a gentle task. ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately describes the main change: updating duplicate key handling from crashing (ValueError) to warning and skipping duplicates in repo loaders.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 fix/export

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@marcinpsk marcinpsk merged commit d3910c3 into main May 27, 2026
7 checks passed
@marcinpsk marcinpsk deleted the fix/export branch May 27, 2026 10:02
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