Conversation
📝 WalkthroughWalkthroughAdds a new pytest marker entry (cluster_health) in pytest.ini and introduces a new test module with three cluster health tests covering node readiness/schedulability, Data Science Cluster Initialization readiness, and DataScienceCluster readiness using existing fixtures and utility functions. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
The following are automatically added/executed:
Available user actions:
Supported labels{'/lgtm', '/hold', '/cherry-pick', '/build-push-pr-image', '/wip', '/verified'} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
pytest.ini (1)
21-21: Tighten grammar and align with existing marker phrasingOther markers use “Mark tests …”. Let’s keep it consistent and fix the verb agreement.
- cluster_health: Tests that verifies that cluster is healthy to begin testing + cluster_health: Mark tests that verify the cluster is healthy before running the suitetests/cluster_health/test_cluster_health.py (2)
16-23: Make expectations explicit and consider adding a short polling window to reduce flakesThe helpers return True or raise; asserting the True return clarifies intent. Also, per utilities/infra.py, these “wait_for_*” helpers don’t actually poll; they check once and raise. That can be flaky immediately after cluster bring-up.
Minimal explicitness:
- wait_for_dsci_status_ready(dsci_resource=dsci_resource) + assert wait_for_dsci_status_ready(dsci_resource=dsci_resource) @@ - wait_for_dsc_status_ready(dsc_resource=dsc_resource) + assert wait_for_dsc_status_ready(dsc_resource=dsc_resource)Optional flake reduction (illustrative only; adapt if you prefer a shared retry utility):
# Example pattern if you decide to poll here: import time from utilities.exceptions import ResourceNotReadyError def _assert_ready_with_retry(check_fn, *, timeout_s=300, interval_s=10): end = time.time() + timeout_s last_err = None while time.time() < end: try: assert check_fn() return except ResourceNotReadyError as e: last_err = e time.sleep(interval_s) raise last_err if last_err else AssertionError("Resource not ready before timeout")
11-11: Type hint portability across Python versionsThe annotation list[Node] requires Python 3.9+. If your supported matrix includes Python 3.8, switch to typing.List.
+from typing import List @@ -def test_cluster_node_healthy(nodes: list[Node]): +def test_cluster_node_healthy(nodes: List[Node]):
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
pytest.ini(1 hunks)tests/cluster_health/test_cluster_health.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/cluster_health/test_cluster_health.py (2)
utilities/infra.py (2)
wait_for_dsci_status_ready(1042-1049)wait_for_dsc_status_ready(1057-1063)tests/conftest.py (3)
nodes(560-561)dsci_resource(410-411)dsc_resource(415-416)
|
/cherry-pick 2.24 |
|
Cherry pick action created PR #552 successfully 🎉! |
|
/cherry-pick 2.23 |
|
Cherry pick action created PR #553 successfully 🎉! |
|
Status of building tag latest: success. |
|
/cherry-pick 2.22 |
|
Cherry pick action created PR #555 successfully 🎉! |
|
/cherry-pick 2.21 |
|
Cherry pick action created PR #566 successfully 🎉! |
* Add cluster health tests (#551) * update docker file --------- Co-authored-by: Debarati Basu-Nag <dbasunag@redhat.com>
4 participants
Description
How Has This Been Tested?
Merge criteria:
Summary by CodeRabbit