chore(workbenches): reduce the workbench spawn timeout to 2mins

[jstourac]

2 minutes should be enough; this helps to reduce the test execution time in cases where the spawning fails

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 timeout configurations to improve testing efficiency.

[github-actions]

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

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

[coderabbitai]

📝 Walkthrough

Walkthrough

Two test parameterizations in a notebook controller spawning test module were updated to use a shorter timeout constant (TIMEOUT_2MIN instead of TIMEOUT_5MIN). No logic, control flow, or test structure changes were made; only the timeout duration values differ.

Changes

Cohort / File(s)	Summary
Test Timeout Configuration tests/workbenches/notebook-controller/test_spawning.py	Updated two test parameterizations to use shorter timeout constant (TIMEOUT_2MIN replacing TIMEOUT_5MIN); parameter objects and test logic remain identical.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	The description includes a brief summary of the change but lacks complete information on testing and unchecked critical checklist items.	Check the 'Locally' and 'Jenkins' testing boxes to confirm test coverage; clarify which testing method was used to validate the timeout reduction.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: reducing the workbench spawn timeout from 5 minutes to 2 minutes.

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

tests/workbenches/notebook-controller/test_spawning.py (1)

80-102: Assertions will raise KeyError if resource keys are missing.

If resources.requests or resources.limits is None, or if the expected keys (cpu, memory) are absent, the test fails with an unhelpful KeyError/TypeError rather than an assertion message. Consider defensive access with .get() or explicit presence checks.

Defensive check pattern

-        assert auth_container.resources.requests["cpu"] == "200m", (
+        requests = auth_container.resources.requests or {}
+        assert requests.get("cpu") == "200m", (
             f"Expected CPU request '200m', got '{auth_container.resources.requests['cpu']}'"
         )

Apply similar pattern for each resource assertion.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/workbenches/notebook-controller/test_spawning.py` around lines 80 -
102, The assertions access auth_container.resources.requests and .limits keys
directly causing KeyError/TypeError when requests/limits or cpu/memory are
missing; update the test around _get_auth_container and notebook_pod to first
assert that auth_container.resources is not None and that resources.requests and
resources.limits are dict-like, then use .get("cpu")/.get("memory") (or safe
dict access) for each check and assert the returned value equals the expected
string with a clear message (e.g., "Auth container cpu request missing" or
"Expected CPU request '200m', got '...'" ) so failures produce informative
assertion messages instead of KeyError.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/workbenches/notebook-controller/test_spawning.py`:
- Line 25: The timeout for notebook pod spawning is set to Timeout.TIMEOUT_2MIN
which is too short compared to the fixture's default pod_ready_timeout
(Timeout.TIMEOUT_10MIN); locate the {"timeout": Timeout.TIMEOUT_2MIN} usage in
tests/workbenches/notebook-controller/test_spawning.py (and the duplicate at the
other occurrence) and either revert it to Timeout.TIMEOUT_10MIN or make it read
from the pod_ready_timeout fixture/config so the test uses the same, validated
spawn timeout; run CI/locally to measure actual spawn durations and document the
chosen minimum timeout in a comment or test docstring.

---

Nitpick comments:
In `@tests/workbenches/notebook-controller/test_spawning.py`:
- Around line 80-102: The assertions access auth_container.resources.requests
and .limits keys directly causing KeyError/TypeError when requests/limits or
cpu/memory are missing; update the test around _get_auth_container and
notebook_pod to first assert that auth_container.resources is not None and that
resources.requests and resources.limits are dict-like, then use
.get("cpu")/.get("memory") (or safe dict access) for each check and assert the
returned value equals the expected string with a clear message (e.g., "Auth
container cpu request missing" or "Expected CPU request '200m', got '...'" ) so
failures produce informative assertion messages instead of KeyError.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: e977b3e3-0c9c-4dee-87c7-ec3b23107fb2

📥 Commits

Reviewing files that changed from the base of the PR and between 41e4488 and 0ce0340.

📒 Files selected for processing (1)

• tests/workbenches/notebook-controller/test_spawning.py

[github-actions]

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