Skip to content

Comments

fix: add exceptions import to template_imports in user_code_parser#2657

Open
rnetser wants to merge 2 commits intomainfrom
fix/issue-2656-duplicate-imports-class-generator
Open

fix: add exceptions import to template_imports in user_code_parser#2657
rnetser wants to merge 2 commits intomainfrom
fix/issue-2656-duplicate-imports-class-generator

Conversation

@rnetser
Copy link
Collaborator

@rnetser rnetser commented Feb 19, 2026

What this PR does

Adds from ocp_resources.exceptions import MissingRequiredArgumentError to the template_imports set in class_generator/parsers/user_code_parser.py.

Why we need it

The Jinja2 template (class_generator_template.j2) generates:

from ocp_resources.exceptions import MissingRequiredArgumentError

But the template_imports set in user_code_parser.py didn't include this pattern. When regenerating resource files with --overwrite, the parser treated this template-generated import as user-added code, preserving it — which then duplicated alongside the freshly rendered template imports.

This was observed in #2647 where CodeRabbit flagged Ruff F811 (redefinition of unused imports).

Which issue(s) this PR fixes

Closes #2656

Test plan

  • Added test_parse_file_with_exceptions_import to verify the new import pattern is recognized as a template import
  • All 8 existing tests in test_user_code_parser.py continue to pass

Summary by CodeRabbit

  • Bug Fixes

    • Fixed import classification to ensure framework imports are properly filtered from user code during parsing.
  • Tests

    • Added test to verify correct handling of exception imports during code parsing, ensuring user imports are preserved while framework imports are filtered.

The template_imports set in user_code_parser.py was missing
"from ocp_resources.exceptions import MissingRequiredArgumentError",
causing duplicate imports when regenerating resource files with --overwrite.

Closes #2656
@coderabbitai
Copy link

coderabbitai bot commented Feb 19, 2026

No actionable comments were generated in the recent review. 🎉


Walkthrough

This PR adds the MissingRequiredArgumentError import pattern from ocp_resources.exceptions to the parser's template_imports set, preventing it from being treated as user-added code during file regeneration. A corresponding test validates the exception import is correctly filtered.

Changes

Cohort / File(s) Summary
Parser Module
class_generator/parsers/user_code_parser.py
Added "from ocp_resources.exceptions import MissingRequiredArgumentError" to the template_imports set to ensure the import is recognized as template-generated and excluded from user imports.
Test Coverage
class_generator/tests/test_user_code_parser.py
Introduced new test method test_parse_file_with_exceptions_import() that verifies template imports (MissingRequiredArgumentError, typing.Any, NamespacedResource) are filtered from user imports while user-level imports (e.g., json) are preserved.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding an exceptions import to template_imports in user_code_parser.
Description check ✅ Passed The description covers all required template sections with clear explanations of what the PR does, why it's needed, which issue it fixes, and the test plan.
Linked Issues check ✅ Passed All requirements from issue #2656 are met: the exceptions import is added to template_imports, a test is added to verify it, and all tests pass.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing issue #2656: one-line addition to template_imports and a test case to verify the fix.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/issue-2656-duplicate-imports-class-generator

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.

@redhat-qe-bot1
Copy link

Report bugs in Issues

Welcome! 🎉

This pull request will be automatically processed with the following features:

🔄 Automatic Actions

  • Reviewer Assignment: Reviewers are automatically assigned based on the OWNERS file in the repository root
  • Size Labeling: PR size labels (XS, S, M, L, XL, XXL) are automatically applied based on changes
  • Issue Creation: Disabled for this repository
  • Branch Labeling: Branch-specific labels are applied to track the target branch
  • Auto-verification: Auto-verified users have their PRs automatically marked as verified
  • Labels: All label categories are enabled (default configuration)

📋 Available Commands

PR Status Management

  • /wip - Mark PR as work in progress (adds WIP: prefix to title)
  • /wip cancel - Remove work in progress status
  • /hold - Block PR merging (approvers only)
  • /hold cancel - Unblock PR merging
  • /verified - Mark PR as verified
  • /verified cancel - Remove verification status
  • /reprocess - Trigger complete PR workflow reprocessing (useful if webhook failed or configuration changed)
  • /regenerate-welcome - Regenerate this welcome message

Review & Approval

  • /lgtm - Approve changes (looks good to me)
  • /approve - Approve PR (approvers only)
  • /automerge - Enable automatic merging when all requirements are met (maintainers and approvers only)
  • /assign-reviewers - Assign reviewers based on OWNERS file
  • /assign-reviewer @username - Assign specific reviewer
  • /check-can-merge - Check if PR meets merge requirements

Testing & Validation

  • /retest tox - Run Python test suite with tox
  • /retest python-module-install - Test Python package installation
  • /retest conventional-title - Validate commit message format
  • /retest all - Run all available tests

Cherry-pick Operations

  • /cherry-pick <branch> - Schedule cherry-pick to target branch when PR is merged
    • Multiple branches: /cherry-pick branch1 branch2 branch3

Label Management

  • /<label-name> - Add a label to the PR
  • /<label-name> cancel - Remove a label from the PR

✅ Merge Requirements

This PR will be automatically approved when the following conditions are met:

  1. Approval: /approve from at least one approver
  2. LGTM Count: Minimum 0 /lgtm from reviewers
  3. Status Checks: All required status checks must pass
  4. No Blockers: No WIP, hold, conflict labels
  5. Verified: PR must be marked as verified (if verification is enabled)

📊 Review Process

Approvers and Reviewers

Approvers:

  • myakove
  • rnetser

Reviewers:

  • myakove
  • rnetser
Available Labels
  • hold
  • verified
  • wip
  • lgtm
  • approve
  • automerge

💡 Tips

  • WIP Status: Use /wip when your PR is not ready for review
  • Verification: The verified label is automatically removed on each new commit
  • Cherry-picking: Cherry-pick labels are processed when the PR is merged
  • Permission Levels: Some commands require approver permissions
  • Auto-verified Users: Certain users have automatic verification and merge privileges

For more information, please refer to the project documentation or contact the maintainers.

@myakove
Copy link
Collaborator

myakove commented Feb 20, 2026

/lgtm
/appove

@myakove
Copy link
Collaborator

myakove commented Feb 20, 2026

/approve

…nto fix/issue-2656-duplicate-imports-class-generator
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: class generator produces duplicate imports when regenerating files

6 participants