Skip to content

feat: add MaaSModelRef resource and deprecate MaaSModel for backward compatibility#2669

Merged
myakove merged 1 commit intoRedHatQE:mainfrom
SB159:feature/new_resource
Mar 6, 2026
Merged

feat: add MaaSModelRef resource and deprecate MaaSModel for backward compatibility#2669
myakove merged 1 commit intoRedHatQE:mainfrom
SB159:feature/new_resource

Conversation

@SB159
Copy link
Copy Markdown
Contributor

@SB159 SB159 commented Mar 6, 2026

…compatibility

Short description:
More details:
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for reviewer:
Bug:

Summary by CodeRabbit

  • New Features

    • MaaSModelRef: New resource class for MAAS OpenDataHub supporting model reference configurations with built-in validation.
  • Deprecations

    • MaaSModel: Deprecation warning now emitted at module import time with guidance on alternatives.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 6, 2026

Walkthrough

Two changes modernize MAAS model handling: a deprecation warning is added to MaaSModel directing users to MaaSModelRef, and a new NamespacedResource subclass MaaSModelRef is introduced with initialization and dictionary conversion logic including validation.

Changes

Cohort / File(s) Summary
Deprecation Warning
ocp_resources/maa_s_model.py
Adds import-time DeprecationWarning directing users to use MaaSModelRef instead of MaaSModel; no API changes.
New MAAS Model Reference
ocp_resources/maas_model_ref.py
Introduces MaaSModelRef NamespacedResource subclass with api_group set to MAAS_OPENDATAHUB_IO, constructor accepting model_ref parameter, to_dict method with conditional validation, and MissingRequiredArgumentError handling when model_ref is required but absent.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is largely empty with only placeholder section headers and no substantive content filled in by the author. Complete all required sections with meaningful details: explain what MaaSModelRef is, why MaaSModel is deprecated, related issues, and any special notes for reviewers.
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main changes: introducing MaaSModelRef and deprecating MaaSModel for backward compatibility, which aligns with the actual code modifications.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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
Copy Markdown

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. Status Checks: All required status checks must pass
  3. No Blockers: No wip, hold, has-conflicts labels and PR must be mergeable (no conflicts)
  4. Verified: PR must be marked as verified

📊 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
Copy Markdown
Collaborator

myakove commented Mar 6, 2026

/lgtm
/approve

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.

🧹 Nitpick comments (1)
ocp_resources/maa_s_model.py (1)

10-15: Module-level deprecation warning fires on every import, even if class is never used.

This warning triggers at import time, which can be noisy in codebases that import this module transitively. Consider moving the deprecation warning into __init__ so it only fires when someone actually instantiates MaaSModel.

Additionally, this modification appears to be within the generated code region (between lines 1 and 52). Per coding guidelines, changes to generated code should be made via the generator tool instead.

♻️ Suggested: Move warning to __init__ for targeted deprecation
-from warnings import warn
-
 from ocp_resources.exceptions import MissingRequiredArgumentError
 from ocp_resources.resource import NamespacedResource
 
-warn(
-    "MaaSModel is deprecated and will be removed in a future release. "
-    "Please use `MaaSModelRef` from maas_model_ref.py instead.",
-    DeprecationWarning,
-    stacklevel=2,
-)
-
 
 class MaaSModel(NamespacedResource):
     """
     MaaSModel is the Schema for the maasmodels API
     """

     api_group: str = NamespacedResource.ApiGroup.MAAS_OPENDATAHUB_IO

     def __init__(
         self,
         model_ref: dict[str, Any] | None = None,
         **kwargs: Any,
     ) -> None:
         r"""
         Args:
             model_ref (dict[str, Any]): ModelRef references the actual model endpoint

         """
+        from warnings import warn
+        warn(
+            "MaaSModel is deprecated and will be removed in a future release. "
+            "Please use `MaaSModelRef` from maas_model_ref.py instead.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
         super().__init__(**kwargs)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ocp_resources/maa_s_model.py` around lines 10 - 15, The module-level
DeprecationWarning call (warn(...)) fires on every import; move that warning
into the MaaSModel.__init__ method so the deprecation only logs when an instance
is created (reference symbols: warn, DeprecationWarning, MaaSModel, __init__).
Do not directly modify the generated-code region (the existing warn call sits
inside the generated block); instead update the generator/template that produces
this file so the warn invocation is emitted inside the MaaSModel.__init__ body
(or add a check in __init__ to call warn once), ensuring the warning is not
triggered at import time.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@ocp_resources/maa_s_model.py`:
- Around line 10-15: The module-level DeprecationWarning call (warn(...)) fires
on every import; move that warning into the MaaSModel.__init__ method so the
deprecation only logs when an instance is created (reference symbols: warn,
DeprecationWarning, MaaSModel, __init__). Do not directly modify the
generated-code region (the existing warn call sits inside the generated block);
instead update the generator/template that produces this file so the warn
invocation is emitted inside the MaaSModel.__init__ body (or add a check in
__init__ to call warn once), ensuring the warning is not triggered at import
time.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: de10666f-9bec-4916-8b0b-1914ed586281

📥 Commits

Reviewing files that changed from the base of the PR and between a09ed75 and e810240.

⛔ Files ignored due to path filters (2)
  • class_generator/schema/__resources-mappings.json.gz is excluded by !**/*.gz, !class_generator/schema/**
  • class_generator/schema/_definitions.json is excluded by !class_generator/schema/**
📒 Files selected for processing (2)
  • ocp_resources/maa_s_model.py
  • ocp_resources/maas_model_ref.py

@SB159
Copy link
Copy Markdown
Contributor Author

SB159 commented Mar 6, 2026

/verified

@myakove
Copy link
Copy Markdown
Collaborator

myakove commented Mar 6, 2026

/check-can-merge

@myakove myakove merged commit 93de61f into RedHatQE:main Mar 6, 2026
7 checks passed
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.

4 participants