feat: Add future warning for api_name in DataVolume constructor, it will default to storage

[ema-aka-young]

Short description:

Add a warning to inform consumers that the default API name will soon change from "pvc" to "storage".

More details:

Implementing step 1 of https://issues.redhat.com/browse/CNV-79539

What this PR does / why we need it:

We need this to achieve a smooth transition to "storage" as the default API name when creating a DataVolume.

Which issue(s) this PR fixes:

Special notes for reviewer:

Bug:

Summary by CodeRabbit

• Chores
  • DataVolume now emits a deprecation warning when the api_name parameter is not explicitly specified. The default value will change from "pvc" to "storage" in a future release. Explicitly set api_name to avoid warnings and ensure compatibility with future versions.

[coderabbitai]

Walkthrough

The DataVolume.__init__ method signature has been modified to accept api_name as an optional parameter with default value None instead of "pvc". When api_name is None, a FutureWarning is emitted and self.api_name defaults to "pvc". The docstring has been updated accordingly, with a note about the future default change to "storage".

Changes

Cohort / File(s)	Summary
DataVolume Deprecation ocp_resources/datavolume.py	Changed api_name parameter default from "pvc" to None in __init__. Added FutureWarning emission when api_name is None, with self.api_name set to "pvc". Updated docstring to document the optional parameter and future default change to "storage".

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: adding a FutureWarning for the api_name parameter in DataVolume and noting the future default change from 'pvc' to 'storage'.
Description check	✅ Passed	The description follows the template with all key sections completed: short description, more details with issue reference, purpose explanation, and notes for reviewers are present.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

_{✏️ 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

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[redhat-qe-bot2]

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.

[coderabbitai]

🧹 Nitpick comments (1)

ocp_resources/datavolume.py (1)

151-161: FutureWarning logic looks good overall.

The warning is clear, actionable, and stacklevel=2 correctly points callers to their own code. One minor note:

Previously, an explicit api_name=None call would leave self.api_name as None, causing to_dict (line 188) to skip populating the pvc/storage spec block entirely. With this change, explicit api_name=None now triggers the warning and silently defaults to "pvc", which is a subtle behavioral difference. If that edge case was never intended to be supported, this is fine — but worth being aware of.

If you want to guard against it, you could use a sentinel instead of None:

Optional: use a sentinel to distinguish "not provided" from explicit None

+_UNSET = object()
+
 def __init__(
     self,
     ...
-    api_name: str | None = None,
+    api_name: str | None | object = _UNSET,
     ...
 ) -> None:
     ...
-    if api_name is None:
+    if api_name is _UNSET:
         warn(
             "The default 'api_name' will change from 'pvc' to 'storage' in a future release. "
             "To maintain existing behavior and suppress this warning, "
             "explicitly set api_name='pvc' in your constructor call.",
             FutureWarning,
             stacklevel=2,
         )
         self.api_name = "pvc"
     else:
         self.api_name = api_name

[ema-aka-young]

🧹 Nitpick comments (1)

ocp_resources/datavolume.py (1)> 151-161: FutureWarning logic looks good overall.

The warning is clear, actionable, and stacklevel=2 correctly points callers to their own code. One minor note:
Previously, an explicit api_name=None call would leave self.api_name as None, causing to_dict (line 188) to skip populating the pvc/storage spec block entirely. With this change, explicit api_name=None now triggers the warning and silently defaults to "pvc", which is a subtle behavioral difference. If that edge case was never intended to be supported, this is fine — but worth being aware of.
If you want to guard against it, you could use a sentinel instead of None:

If I'm getting this correctly, None isn't a legitimate value for api_name, so setting explicitly (or not) api_name=None should default to 'pvc' as it was before this change and trigger the warning. Therefore, I don't think we would benefit from using the sentinel pattern in this case. Happy to hear different feedback from reviewers.

[ema-aka-young]

/verified
through ipython and --with-editable ../openshift-python-wrapper

[myakove]

/lgtm
/approve