Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update cluter list according to daemon and cluster statuses separation #1527

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

update cluter list according to daemon and cluster statuses separation #1527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions runhouse/cli_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ def create_output_table(
# Add columns to the table
table.add_column("Name", justify="left", no_wrap=True)
table.add_column("Cluster Type", justify="left", no_wrap=True)
table.add_column("Status", justify="left")
table.add_column("Cluster Status", justify="left")
table.add_column("Daemon Status", justify="left")
table.add_column("Last Active (UTC)", justify="left")

return table
Expand All @@ -105,7 +106,8 @@ def add_cluster_as_table_row(table: Table, rh_cluster: dict):
table.add_row(
rh_cluster.get("Name"),
rh_cluster.get("Cluster Type"),
rh_cluster.get("Status"),
rh_cluster.get("Cluster Status"),
rh_cluster.get("Daemon Status"),
rh_cluster.get("Last Active (UTC)"),
)

Expand All @@ -123,8 +125,11 @@ def add_clusters_to_output_table(table: Table, clusters: List[Dict]):
0
] # The split is required to remove the offset (according to UTC)
rh_cluster["Last Active (UTC)"] = last_active_at_no_offset
rh_cluster["Status"] = ClusterStatusColors.get_status_color(
rh_cluster.get("Status")
rh_cluster["Cluster Status"] = ClusterStatusColors.get_status_color(
rh_cluster.get("Cluster Status")
)
rh_cluster["Daemon Status"] = ClusterStatusColors.get_status_color(
rh_cluster.get("Daemon Status")
)

table = add_cluster_as_table_row(table, rh_cluster)
Expand Down
8 changes: 3 additions & 5 deletions runhouse/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ def cluster_list(
help="Time duration to filter on. Minimum allowable filter is 1 minute. You may filter by seconds (s), "
"minutes (m), hours (h) or days (s). Examples: 30s, 15m, 2h, 3d.",
),
cluster_status: Optional[ClusterStatus] = typer.Option(
status: Optional[ClusterStatus] = typer.Option(
None,
"--status",
help="Cluster status to filter on.",
Expand Down Expand Up @@ -282,16 +282,14 @@ def cluster_list(
)
return

clusters = Cluster.list(
show_all=show_all, since=since, status=cluster_status, force=force
)
clusters = Cluster.list(show_all=show_all, since=since, status=status, force=force)

den_clusters = clusters.get("den_clusters", None)
running_clusters = (
[
den_cluster
for den_cluster in den_clusters
if den_cluster.get("Status") == ClusterStatus.RUNNING
if den_cluster.get("Cluster Status") == ClusterStatus.RUNNING
]
if den_clusters
else None
Expand Down
16 changes: 9 additions & 7 deletions runhouse/resources/hardware/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,12 +395,12 @@ def parse_filters(since: str, cluster_status: Union[str, ClusterStatus]):
def get_clusters_from_den(cluster_filters: dict, force: bool):
get_clusters_params = {"resource_type": "cluster", "folder": rns_client.username}

if "cluster_status" in cluster_filters and any(
cluster_filters.get("cluster_status") == daemon_status
for daemon_status in RunhouseDaemonStatus
if (
"cluster_status" in cluster_filters
and cluster_filters["cluster_status"] == ClusterStatus.TERMINATED
):
# Include the relevant daemon status for the filter
cluster_filters["daemon_status"] = cluster_filters.get("cluster_status")
cluster_filters["daemon_status"] = RunhouseDaemonStatus.TERMINATED

# If "all" filter is specified load all clusters (no filters are added to get_clusters_params)
if cluster_filters and "all" not in cluster_filters.keys():
Expand Down Expand Up @@ -487,6 +487,7 @@ def get_running_and_not_running_clusters(clusters: list):
cluster_type = den_cluster.get("data").get("resource_subtype")
cluster_status = den_cluster.get("cluster_status")
cluster_status = cluster_status or ClusterStatus.UNKNOWN.value
daemon_status = den_cluster.get("daemon_status", RunhouseDaemonStatus.UNKNOWN)

# The split is required to remove milliseconds and the offset (according to UTC) from the timestamp.
# (cluster_status_last_checked is in the following format: YYYY-MM-DD HH:MM:SS.ssssss±HH:MM)
Expand All @@ -507,7 +508,8 @@ def get_running_and_not_running_clusters(clusters: list):
cluster_info = {
"Name": cluster_name,
"Cluster Type": cluster_type,
"Status": cluster_status,
"Cluster Status": cluster_status,
"Daemon Status": daemon_status,
"Last Active (UTC)": last_active_at,
}

Expand All @@ -516,10 +518,10 @@ def get_running_and_not_running_clusters(clusters: list):
else:
down_clusters.append(cluster_info)

# Sort clusters by the 'Last Active (UTC)' and 'Status' column
# Sort clusters by the 'Last Active (UTC)' and 'Cluster Status' column
down_clusters = sorted(
down_clusters,
key=lambda x: (x["Last Active (UTC)"], x["Status"]),
key=lambda x: (x["Last Active (UTC)"], x["Cluster Status"]),
reverse=True,
)

Expand Down
58 changes: 43 additions & 15 deletions tests/test_resources/test_clusters/test_cluster.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -993,7 +993,7 @@ def test_cluster_list_default_pythonic(self, cluster):
default_clusters = Cluster.list().get("den_clusters", {})
assert len(default_clusters) > 0
assert [
den_cluster.get("Status") == ClusterStatus.RUNNING
den_cluster.get("Cluster Status") == ClusterStatus.RUNNING
for den_cluster in default_clusters
]
assert any(
Expand Down Expand Up @@ -1028,7 +1028,7 @@ def test_cluster_list_all_pythonic(self, cluster):

all_clusters = Cluster.list(show_all=True).get("den_clusters", {})
present_statuses = set(
[den_cluster.get("Status") for den_cluster in all_clusters]
[den_cluster.get("Cluster Status") for den_cluster in all_clusters]
)
assert len(present_statuses) > 1
assert ClusterStatus.RUNNING in present_statuses
Expand All @@ -1039,7 +1039,7 @@ def test_cluster_list_all_pythonic(self, cluster):
for den_cluster in all_clusters
if den_cluster.get("Name") == cluster.name
][0]
assert test_cluster.get("Status") == ClusterStatus.RUNNING
assert test_cluster.get("Cluster Status") == ClusterStatus.RUNNING

@pytest.mark.level("local")
@pytest.mark.clustertest
Expand All @@ -1060,10 +1060,10 @@ def test_cluster_list_status_pythonic(self, cluster):
# check that filtered requests contains only specific status
filtered_clusters = Cluster.list(status=status).get("den_clusters", {})
if filtered_clusters:
filtered_statuses = set(
[cluster.get("Status") for cluster in filtered_clusters]
filtered_cluster_statuses = set(
[cluster.get("Cluster Status") for cluster in filtered_clusters]
)
assert filtered_statuses == {status}
assert filtered_cluster_statuses == {status}

@pytest.mark.level("local")
@pytest.mark.clustertest
Expand Down Expand Up @@ -1144,7 +1144,13 @@ def test_cluster_list_cmd_output_no_filters(self, capsys, cluster):
assert re.search(regex, cmd_stdout)

# testing that the table column names is printed correctly
col_names = ["┃ Name", "┃ Cluster Type", "┃ Status", "┃ Last Active (UTC)"]
col_names = [
"┃ Name",
"┃ Cluster Type",
"┃ Cluster Status",
"┃ Daemon Status",
"┃ Last Active (UTC)",
]
for name in col_names:
assert name in cmd_stdout
assert (
Expand All @@ -1155,7 +1161,10 @@ def test_cluster_list_cmd_output_no_filters(self, capsys, cluster):

@pytest.mark.level("local")
@pytest.mark.clustertest
def test_cluster_list_cmd_output_with_filters(self, capsys, cluster):
def test_cluster_list_cmd_output_with_filters(
self, capsys, docker_cluster_pk_ssh_no_auth
):
cluster = docker_cluster_pk_ssh_no_auth

import re
import subprocess
Expand Down Expand Up @@ -1200,7 +1209,8 @@ def test_cluster_list_cmd_output_with_filters(self, capsys, cluster):
col_names = [
"┃ Name",
"┃ Cluster Type",
"┃ Status",
"┃ Cluster Status",
"┃ Daemon Status",
"┃ Last Active (UTC)",
]
for name in col_names:
Expand All @@ -1214,13 +1224,31 @@ def test_cluster_list_cmd_output_with_filters(self, capsys, cluster):
if status == ClusterStatus.RUNNING:
assert cluster.name in cmd_stdout

# Check other statuses not found in output
cmd_stdout = cmd_stdout.replace("Running:", "")
statuses = [s.lower() for s in list(ClusterStatus.__members__.keys())]
statuses.remove(status)
# clean-up the cmd output, and get only the records that describes the clusters info, for example:
# │ gcp-cpu │ OnDemandCluster │ Running │ Running │ 12/05/2024, 02:49:56 │
cmd_stdout_clusters_info = [
cluster_info.split("│")
for cluster_info in cmd_stdout.split("\n")
if "│" in cluster_info
]

# remove redundant spaces from each cluster record in cmd_stdout_clusters_info
cmd_stdout_clusters_info = [
[
cluster_info_val.strip()
for cluster_info_val in cluster_info
if cluster_info_val != ""
]
for cluster_info in cmd_stdout_clusters_info
]

for status in statuses:
assert status.capitalize() not in cmd_stdout
for cluster_info in cmd_stdout_clusters_info:
# we know that the clusters info is printed in the following order:
# ┃ Name ┃ Cluster Type ┃ Cluster Status ┃ Daemon Status ┃ Last Active (UTC) ┃
# therefore, for each cluster record that is printed we are checking the value in the
# 3ed index, which is the value of 'Cluster Status'. We want to check that this values match the
# status we are filtering on.
assert status.capitalize() == cluster_info[2]

@pytest.mark.level("local")
@pytest.mark.clustertest
Expand Down
Loading