diff --git a/runhouse/cli_utils.py b/runhouse/cli_utils.py index bba2df708..0328396da 100644 --- a/runhouse/cli_utils.py +++ b/runhouse/cli_utils.py @@ -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 @@ -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)"), ) @@ -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) diff --git a/runhouse/main.py b/runhouse/main.py index 64bd98e89..517492a86 100644 --- a/runhouse/main.py +++ b/runhouse/main.py @@ -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.", @@ -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 diff --git a/runhouse/resources/hardware/utils.py b/runhouse/resources/hardware/utils.py index ce8f044b8..d8fec1ba1 100644 --- a/runhouse/resources/hardware/utils.py +++ b/runhouse/resources/hardware/utils.py @@ -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(): @@ -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) @@ -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, } @@ -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, ) diff --git a/tests/test_resources/test_clusters/test_cluster.py b/tests/test_resources/test_clusters/test_cluster.py index 91fe01e95..a40a95413 100644 --- a/tests/test_resources/test_clusters/test_cluster.py +++ b/tests/test_resources/test_clusters/test_cluster.py @@ -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( @@ -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 @@ -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 @@ -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 @@ -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 ( @@ -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 @@ -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: @@ -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