Skip to content

fix(workbenches): explicitly define the ocp client#1167

Merged
jstourac merged 4 commits intoopendatahub-io:mainfrom
jstourac:workbenchesFutureWarning
Mar 5, 2026
Merged

fix(workbenches): explicitly define the ocp client#1167
jstourac merged 4 commits intoopendatahub-io:mainfrom
jstourac:workbenchesFutureWarning

Conversation

@jstourac
Copy link
Copy Markdown
Member

@jstourac jstourac commented Mar 4, 2026

This is to get rid of the:

.../.venv/lib/python3.13/site-packages/ocp_resources/resource.py:1614: FutureWarning: 'client' \
arg will be mandatory in the next major release. `config_file` and `context` args will be removed.

Pull Request

Summary

Related Issues

  • Fixes:
  • JIRA:

How it has been tested

  • Locally
  • Jenkins

Additional Requirements

  • If this PR introduces a new test image, did you create a PR to mirror it in disconnected environment?
  • If this PR introduces new marker(s)/adds a new component, was relevant ticket created to update relevant Jenkins job?

Summary by CodeRabbit

  • Tests
    • Updated test fixture to initialize notebooks with unprivileged access scope, ensuring comprehensive testing of notebook functionality under restricted permission conditions.

This is to get rid of the:
```
.../.venv/lib/python3.13/site-packages/ocp_resources/resource.py:1614: FutureWarning: 'client' \
arg will be mandatory in the next major release. `config_file` and `context` args will be removed.
```
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 4, 2026

The following are automatically added/executed:

  • PR size label.
  • Run pre-commit
  • Run tox
  • Add PR author as the PR assignee
  • Build image based on the PR

Available user actions:

  • To mark a PR as WIP, add /wip in a comment. To remove it from the PR comment /wip cancel to the PR.
  • To block merging of a PR, add /hold in a comment. To un-block merging of PR comment /hold cancel.
  • To mark a PR as approved, add /lgtm in a comment. To remove, add /lgtm cancel.
    lgtm label removed on each new commit push.
  • To mark PR as verified comment /verified to the PR, to un-verify comment /verified cancel to the PR.
    verified label removed on each new commit push.
  • To Cherry-pick a merged PR /cherry-pick <target_branch_name> to the PR. If <target_branch_name> is valid,
    and the current PR is merged, a cherry-picked PR would be created and linked to the current PR.
  • To build and push image to quay, add /build-push-pr-image in a comment. This would create an image with tag
    pr-<pr_number> to quay repository. This image tag, however would be deleted on PR merge or close action.
Supported labels

{'/hold', '/cherry-pick', '/build-push-pr-image', '/lgtm', '/wip', '/verified'}

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 4, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 41411064-ea59-44fc-b25d-fb6616690685

📥 Commits

Reviewing files that changed from the base of the PR and between 39d3462 and 6cc223f.

📒 Files selected for processing (1)
  • tests/workbenches/conftest.py

📝 Walkthrough

Walkthrough

The default_notebook test fixture in tests/workbenches/conftest.py is modified to accept an unprivileged_client parameter and pass it explicitly to the Notebook constructor, replacing the previous call that relied on default client initialization.

Changes

Cohort / File(s) Summary
Test Fixture Update
tests/workbenches/conftest.py
Added unprivileged_client: DynamicClient parameter to default_notebook fixture and updated Notebook initialization to explicitly pass the unprivileged client alongside the kind_dict argument.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(workbenches): explicitly define the ocp client' directly describes the main change—adding an explicit OCP client parameter to the Notebook fixture to address a FutureWarning.
Description check ✅ Passed The description explains the FutureWarning being addressed and is mostly complete, though the Summary section and Related Issues are incomplete, and the pull request template structure is partially filled.
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 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.

@dbasunag dbasunag enabled auto-merge (squash) March 4, 2026 18:44
Copy link
Copy Markdown
Contributor

@sheltoncyril sheltoncyril left a comment

Choose a reason for hiding this comment

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

/lgtm

@jstourac jstourac merged commit 7a049ac into opendatahub-io:main Mar 5, 2026
9 checks passed
@jstourac jstourac deleted the workbenchesFutureWarning branch March 5, 2026 16:03
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 5, 2026

Status of building tag latest: success.
Status of pushing tag latest to image registry: success.

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.

5 participants