From d212a3815d36da78fc7b3acf2dfdd114669f6b0b Mon Sep 17 00:00:00 2001 From: ohadchaet Date: Thu, 29 Dec 2022 09:21:56 +0200 Subject: [PATCH 01/18] feat: Adds "head" flag to `flow-run logs` - If head is provided, the first N logs are returned - Otherwise, all the logs are returned linked to issue #7998 --- src/prefect/cli/flow_run.py | 23 ++++++++++- tests/cli/test_flow_run.py | 80 +++++++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 2 deletions(-) diff --git a/src/prefect/cli/flow_run.py b/src/prefect/cli/flow_run.py index 808201f75a2c..108b0c2b69ec 100644 --- a/src/prefect/cli/flow_run.py +++ b/src/prefect/cli/flow_run.py @@ -139,13 +139,25 @@ async def cancel(id: UUID): @flow_run_app.command() -async def logs(id: UUID): +async def logs( + id: UUID, + head: int = typer.Option( + None, + help="Number of lines to show from the head of the log", + ), +): """ View logs for a flow run. """ + # Pagination - API returns max 200 lines at a time page_size = 200 offset = 0 more_logs = True + + # If head is specified, we need to stop after we've retrieved enough lines + remaining_logs = head + num_logs_returned = 0 + log_filter = LogFilter(flow_run_id={"any_": [id]}) async with get_client() as client: @@ -156,9 +168,13 @@ async def logs(id: UUID): exit_with_error(f"Flow run {str(id)!r} not found!") while more_logs: + num_logs_to_return_from_page = ( + page_size if remaining_logs is None else min(page_size, remaining_logs) + ) + # Get the next page of logs page_logs = await client.read_logs( - log_filter=log_filter, limit=page_size, offset=offset + log_filter=log_filter, limit=num_logs_to_return_from_page, offset=offset ) # Print the logs @@ -169,6 +185,9 @@ async def logs(id: UUID): soft_wrap=True, ) + # Update the number of logs retrieved + num_logs_returned += num_logs_to_return_from_page + if len(page_logs) == page_size: offset += page_size else: diff --git a/tests/cli/test_flow_run.py b/tests/cli/test_flow_run.py index 7b8f0311e373..7c07133545fc 100644 --- a/tests/cli/test_flow_run.py +++ b/tests/cli/test_flow_run.py @@ -372,3 +372,83 @@ async def test_when_flow_run_not_found_then_exit_with_error(self, state): expected_code=1, expected_output_contains=f"Flow run '{bad_id}' not found!\n", ) + + @pytest.mark.parametrize("state", [Completed, Failed, Crashed, Cancelled]) + async def test_when_num_logs_smaller_than_page_size_with_head_then_no_pagination( + self, orion_client, state + ): + # Given + flow_run = await orion_client.create_flow_run( + name="scheduled_flow_run", flow=hello_flow, state=state() + ) + + # Create enough flow run logs to result in pagination (page_size > 200) + logs = [ + LogCreate( + name="prefect.flow_runs", + level=20, + message=f"Log {i} from flow_run {flow_run.id}.", + timestamp=datetime.now(tz=timezone.utc), + flow_run_id=flow_run.id, + ) + for i in range(self.PAGE_SIZE + 1) + ] + await orion_client.create_logs(logs) + + # When/Then + await run_sync_in_worker_thread( + invoke_and_assert, + command=[ + "flow-run", + "logs", + str(flow_run.id), + "--head", + "10", + ], + expected_code=0, + expected_output_contains=[ + f"Flow run '{flow_run.name}' - Log {i} from flow_run {flow_run.id}." + for i in range(10) + ], + expected_line_count=10, + ) + + @pytest.mark.parametrize("state", [Completed, Failed, Crashed, Cancelled]) + async def test_when_num_logs_greater_than_page_size_with_head_then_pagination( + self, orion_client, state + ): + # Given + flow_run = await orion_client.create_flow_run( + name="scheduled_flow_run", flow=hello_flow, state=state() + ) + + # Create enough flow run logs to result in pagination (page_size > 200) + logs = [ + LogCreate( + name="prefect.flow_runs", + level=20, + message=f"Log {i} from flow_run {flow_run.id}.", + timestamp=datetime.now(tz=timezone.utc), + flow_run_id=flow_run.id, + ) + for i in range(self.PAGE_SIZE + 1) + ] + await orion_client.create_logs(logs) + + # When/Then + await run_sync_in_worker_thread( + invoke_and_assert, + command=[ + "flow-run", + "logs", + str(flow_run.id), + "--head", + "300", + ], + expected_code=0, + expected_output_contains=[ + f"Flow run '{flow_run.name}' - Log {i} from flow_run {flow_run.id}." + for i in range(self.PAGE_SIZE + 1) + ], + expected_line_count=self.PAGE_SIZE + 1, + ) From 8c4f2c1ebc935fba93cb94fba25d4d549364fef1 Mon Sep 17 00:00:00 2001 From: ohadchaet Date: Thu, 29 Dec 2022 10:06:29 +0200 Subject: [PATCH 02/18] fix: Provides test with correct head value --- tests/cli/test_flow_run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cli/test_flow_run.py b/tests/cli/test_flow_run.py index 7c07133545fc..20f4667f41fe 100644 --- a/tests/cli/test_flow_run.py +++ b/tests/cli/test_flow_run.py @@ -443,7 +443,7 @@ async def test_when_num_logs_greater_than_page_size_with_head_then_pagination( "logs", str(flow_run.id), "--head", - "300", + self.PAGE_SIZE + 1, ], expected_code=0, expected_output_contains=[ From 85e94bd551f35572bd7a604c76597c269b01770a Mon Sep 17 00:00:00 2001 From: ohadchaet Date: Thu, 29 Dec 2022 19:18:00 +0200 Subject: [PATCH 03/18] refactor: renames remaining_logs to user_specified_num_logs --- src/prefect/cli/flow_run.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/prefect/cli/flow_run.py b/src/prefect/cli/flow_run.py index 108b0c2b69ec..b4eecf55e73a 100644 --- a/src/prefect/cli/flow_run.py +++ b/src/prefect/cli/flow_run.py @@ -153,10 +153,10 @@ async def logs( page_size = 200 offset = 0 more_logs = True + num_logs_returned = 0 # If head is specified, we need to stop after we've retrieved enough lines - remaining_logs = head - num_logs_returned = 0 + user_specified_num_logs = head log_filter = LogFilter(flow_run_id={"any_": [id]}) @@ -169,7 +169,9 @@ async def logs( while more_logs: num_logs_to_return_from_page = ( - page_size if remaining_logs is None else min(page_size, remaining_logs) + page_size + if user_specified_num_logs is None + else min(page_size, user_specified_num_logs) ) # Get the next page of logs From 70c7e0e8b1cd1971281ee4aba8719748472dcab0 Mon Sep 17 00:00:00 2001 From: ohadchaet Date: Thu, 29 Dec 2022 20:02:54 +0200 Subject: [PATCH 04/18] feat: Uses a flow_run_factory fixture --- tests/cli/test_flow_run.py | 93 +++++++++++--------------------------- 1 file changed, 26 insertions(+), 67 deletions(-) diff --git a/tests/cli/test_flow_run.py b/tests/cli/test_flow_run.py index 20f4667f41fe..60c460f02d69 100644 --- a/tests/cli/test_flow_run.py +++ b/tests/cli/test_flow_run.py @@ -279,19 +279,13 @@ def test_wrong_id_exits_with_error(self): ) -class TestFlowRunLogs: - PAGE_SIZE = 200 - - @pytest.mark.parametrize("state", [Completed, Failed, Crashed, Cancelled]) - async def test_when_num_logs_smaller_than_page_size_then_no_pagination( - self, orion_client, state - ): - # Given +@pytest.fixture() +def flow_run_factory(orion_client): + async def create_flow_run(num_logs: int): flow_run = await orion_client.create_flow_run( - name="scheduled_flow_run", flow=hello_flow, state=state() + name="scheduled_flow_run", flow=hello_flow ) - # Create not enough flow run logs to result in pagination (page_size <= 200) logs = [ LogCreate( name="prefect.flow_runs", @@ -300,10 +294,24 @@ async def test_when_num_logs_smaller_than_page_size_then_no_pagination( timestamp=datetime.now(tz=timezone.utc), flow_run_id=flow_run.id, ) - for i in range(self.PAGE_SIZE - 1) + for i in range(num_logs) ] await orion_client.create_logs(logs) + return flow_run + + return create_flow_run + + +class TestFlowRunLogs: + PAGE_SIZE = 200 + + async def test_when_num_logs_smaller_than_page_size_then_no_pagination( + self, flow_run_factory + ): + # Given + flow_run = await flow_run_factory(num_logs=self.PAGE_SIZE - 1) + # When/Then await run_sync_in_worker_thread( invoke_and_assert, @@ -319,27 +327,11 @@ async def test_when_num_logs_smaller_than_page_size_then_no_pagination( ], ) - @pytest.mark.parametrize("state", [Completed, Failed, Crashed, Cancelled]) async def test_when_num_logs_greater_than_page_size_then_pagination( - self, orion_client, state + self, flow_run_factory ): # Given - flow_run = await orion_client.create_flow_run( - name="scheduled_flow_run", flow=hello_flow, state=state() - ) - - # Create enough flow run logs to result in pagination (page_size > 200) - logs = [ - LogCreate( - name="prefect.flow_runs", - level=20, - message=f"Log {i} from flow_run {flow_run.id}.", - timestamp=datetime.now(tz=timezone.utc), - flow_run_id=flow_run.id, - ) - for i in range(self.PAGE_SIZE + 1) - ] - await orion_client.create_logs(logs) + flow_run = await flow_run_factory(num_logs=self.PAGE_SIZE + 1) # When/Then await run_sync_in_worker_thread( @@ -356,8 +348,7 @@ async def test_when_num_logs_greater_than_page_size_then_pagination( ], ) - @pytest.mark.parametrize("state", [Completed, Failed, Crashed, Cancelled]) - async def test_when_flow_run_not_found_then_exit_with_error(self, state): + async def test_when_flow_run_not_found_then_exit_with_error(self, flow_run_factory): # Given bad_id = str(uuid4()) @@ -373,27 +364,11 @@ async def test_when_flow_run_not_found_then_exit_with_error(self, state): expected_output_contains=f"Flow run '{bad_id}' not found!\n", ) - @pytest.mark.parametrize("state", [Completed, Failed, Crashed, Cancelled]) async def test_when_num_logs_smaller_than_page_size_with_head_then_no_pagination( - self, orion_client, state + self, flow_run_factory ): # Given - flow_run = await orion_client.create_flow_run( - name="scheduled_flow_run", flow=hello_flow, state=state() - ) - - # Create enough flow run logs to result in pagination (page_size > 200) - logs = [ - LogCreate( - name="prefect.flow_runs", - level=20, - message=f"Log {i} from flow_run {flow_run.id}.", - timestamp=datetime.now(tz=timezone.utc), - flow_run_id=flow_run.id, - ) - for i in range(self.PAGE_SIZE + 1) - ] - await orion_client.create_logs(logs) + flow_run = await flow_run_factory(num_logs=self.PAGE_SIZE + 1) # When/Then await run_sync_in_worker_thread( @@ -413,27 +388,11 @@ async def test_when_num_logs_smaller_than_page_size_with_head_then_no_pagination expected_line_count=10, ) - @pytest.mark.parametrize("state", [Completed, Failed, Crashed, Cancelled]) async def test_when_num_logs_greater_than_page_size_with_head_then_pagination( - self, orion_client, state + self, flow_run_factory ): # Given - flow_run = await orion_client.create_flow_run( - name="scheduled_flow_run", flow=hello_flow, state=state() - ) - - # Create enough flow run logs to result in pagination (page_size > 200) - logs = [ - LogCreate( - name="prefect.flow_runs", - level=20, - message=f"Log {i} from flow_run {flow_run.id}.", - timestamp=datetime.now(tz=timezone.utc), - flow_run_id=flow_run.id, - ) - for i in range(self.PAGE_SIZE + 1) - ] - await orion_client.create_logs(logs) + flow_run = await flow_run_factory(num_logs=self.PAGE_SIZE + 1) # When/Then await run_sync_in_worker_thread( From bef6e0875eab77a4356c1d98f8949edc259b35c8 Mon Sep 17 00:00:00 2001 From: ohadchaet Date: Fri, 30 Dec 2022 16:55:43 +0200 Subject: [PATCH 05/18] refactor: uses constants --- src/prefect/cli/flow_run.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/prefect/cli/flow_run.py b/src/prefect/cli/flow_run.py index b4eecf55e73a..6a15562487c6 100644 --- a/src/prefect/cli/flow_run.py +++ b/src/prefect/cli/flow_run.py @@ -28,6 +28,9 @@ ) app.add_typer(flow_run_app, aliases=["flow-runs"]) +LOGS_PAGE_SIZE = 200 +LOGS_HEAD_AND_TAIL_DEFAULT_NUM_LINES = 50 + @flow_run_app.command() async def inspect(id: UUID): @@ -141,16 +144,18 @@ async def cancel(id: UUID): @flow_run_app.command() async def logs( id: UUID, - head: int = typer.Option( - None, - help="Number of lines to show from the head of the log", + # Boolean head flag + head: bool = typer.Option( + False, + "--head", + "-h", + help="Show the first 10 lines of logs instead of the last 10 lines", ), ): """ View logs for a flow run. """ # Pagination - API returns max 200 lines at a time - page_size = 200 offset = 0 more_logs = True num_logs_returned = 0 @@ -169,9 +174,9 @@ async def logs( while more_logs: num_logs_to_return_from_page = ( - page_size + LOGS_PAGE_SIZE if user_specified_num_logs is None - else min(page_size, user_specified_num_logs) + else min(LOGS_PAGE_SIZE, user_specified_num_logs) ) # Get the next page of logs @@ -190,8 +195,8 @@ async def logs( # Update the number of logs retrieved num_logs_returned += num_logs_to_return_from_page - if len(page_logs) == page_size: - offset += page_size + if len(page_logs) == LOGS_PAGE_SIZE: + offset += LOGS_PAGE_SIZE else: # No more logs to show, exit more_logs = False From d59851d395212295d20d8df6e0ba15ba97afe878 Mon Sep 17 00:00:00 2001 From: ohadchaet Date: Fri, 30 Dec 2022 17:03:25 +0200 Subject: [PATCH 06/18] feat: Adds --num-lines flag - flow-run logs --head -> Returns 50 logs - flow-run logs --head --num-lines 10 -> returns 10 logs --- src/prefect/cli/flow_run.py | 14 ++++-- tests/cli/test_flow_run.py | 92 +++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 4 deletions(-) diff --git a/src/prefect/cli/flow_run.py b/src/prefect/cli/flow_run.py index 6a15562487c6..597c0e6188e3 100644 --- a/src/prefect/cli/flow_run.py +++ b/src/prefect/cli/flow_run.py @@ -29,7 +29,7 @@ app.add_typer(flow_run_app, aliases=["flow-runs"]) LOGS_PAGE_SIZE = 200 -LOGS_HEAD_AND_TAIL_DEFAULT_NUM_LINES = 50 +LOGS_HEAD_DEFAULT_NUM_LINES = 50 @flow_run_app.command() @@ -144,12 +144,18 @@ async def cancel(id: UUID): @flow_run_app.command() async def logs( id: UUID, - # Boolean head flag head: bool = typer.Option( False, "--head", "-h", - help="Show the first 10 lines of logs instead of the last 10 lines", + help=f"Show the first {LOGS_HEAD_DEFAULT_NUM_LINES} lines of logs instead of the last 10 lines", + ), + num_lines: int = typer.Option( + LOGS_HEAD_DEFAULT_NUM_LINES, + "--num-lines", + "-n", + help=f"Number of lines to show when using the --head flag", + min=1, ), ): """ @@ -161,7 +167,7 @@ async def logs( num_logs_returned = 0 # If head is specified, we need to stop after we've retrieved enough lines - user_specified_num_logs = head + user_specified_num_logs = num_lines if head else None log_filter = LogFilter(flow_run_id={"any_": [id]}) diff --git a/tests/cli/test_flow_run.py b/tests/cli/test_flow_run.py index 60c460f02d69..d8062e8d7dea 100644 --- a/tests/cli/test_flow_run.py +++ b/tests/cli/test_flow_run.py @@ -378,6 +378,7 @@ async def test_when_num_logs_smaller_than_page_size_with_head_then_no_pagination "logs", str(flow_run.id), "--head", + "--num-lines", "10", ], expected_code=0, @@ -402,6 +403,7 @@ async def test_when_num_logs_greater_than_page_size_with_head_then_pagination( "logs", str(flow_run.id), "--head", + "--num-lines", self.PAGE_SIZE + 1, ], expected_code=0, @@ -411,3 +413,93 @@ async def test_when_num_logs_greater_than_page_size_with_head_then_pagination( ], expected_line_count=self.PAGE_SIZE + 1, ) + + async def test_when_head_without_num_lines_then_return_50_lines( + self, flow_run_factory + ): + # Given + flow_run = await flow_run_factory(num_logs=self.PAGE_SIZE + 1) + + # When/Then + await run_sync_in_worker_thread( + invoke_and_assert, + command=[ + "flow-run", + "logs", + str(flow_run.id), + "--head", + ], + expected_code=0, + expected_output_contains=[ + f"Flow run '{flow_run.name}' - Log {i} from flow_run {flow_run.id}." + for i in range(50) + ], + expected_line_count=50, + ) + + async def test_h_and_n_shortcuts_for_head_and_num_lines(self, flow_run_factory): + # Given + flow_run = await flow_run_factory(num_logs=self.PAGE_SIZE + 1) + + # When/Then + await run_sync_in_worker_thread( + invoke_and_assert, + command=[ + "flow-run", + "logs", + str(flow_run.id), + "-h", + "-n", + "10", + ], + expected_code=0, + expected_output_contains=[ + f"Flow run '{flow_run.name}' - Log {i} from flow_run {flow_run.id}." + for i in range(10) + ], + expected_line_count=10, + ) + + async def test_when_num_lines_passed_without_head_then_ignore_num_lines( + self, flow_run_factory + ): + # Given + flow_run = await flow_run_factory(num_logs=self.PAGE_SIZE + 1) + + # When/Then + await run_sync_in_worker_thread( + invoke_and_assert, + command=[ + "flow-run", + "logs", + str(flow_run.id), + "--num-lines", + "10", + ], + expected_code=0, + expected_output_contains=[ + f"Flow run '{flow_run.name}' - Log {i} from flow_run {flow_run.id}." + for i in range(self.PAGE_SIZE + 1) + ], + expected_line_count=self.PAGE_SIZE + 1, + ) + + async def test_when_num_lines_is_smaller_than_one_then_exit_with_error( + self, flow_run_factory + ): + # Given + flow_run = await flow_run_factory(num_logs=self.PAGE_SIZE + 1) + + # When/Then + await run_sync_in_worker_thread( + invoke_and_assert, + command=[ + "flow-run", + "logs", + str(flow_run.id), + "--num-lines", + "0", + ], + expected_code=2, + expected_output_contains="Invalid value for '--num-lines' / '-n': 0 is not in the range x>=1.", + ) From acdc5db77351bcf08c65decd44a4c1fd17719138 Mon Sep 17 00:00:00 2001 From: ohadchaet Date: Fri, 30 Dec 2022 17:13:02 +0200 Subject: [PATCH 07/18] fix: Default head value is 20 lines --- src/prefect/cli/flow_run.py | 2 +- tests/cli/test_flow_run.py | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/prefect/cli/flow_run.py b/src/prefect/cli/flow_run.py index 597c0e6188e3..6d52c9e0c4ca 100644 --- a/src/prefect/cli/flow_run.py +++ b/src/prefect/cli/flow_run.py @@ -29,7 +29,7 @@ app.add_typer(flow_run_app, aliases=["flow-runs"]) LOGS_PAGE_SIZE = 200 -LOGS_HEAD_DEFAULT_NUM_LINES = 50 +LOGS_HEAD_DEFAULT_NUM_LINES = 20 @flow_run_app.command() diff --git a/tests/cli/test_flow_run.py b/tests/cli/test_flow_run.py index d8062e8d7dea..1bf6ccd44d1c 100644 --- a/tests/cli/test_flow_run.py +++ b/tests/cli/test_flow_run.py @@ -5,6 +5,7 @@ import prefect.exceptions from prefect import flow +from prefect.cli.flow_run import LOGS_HEAD_DEFAULT_NUM_LINES from prefect.orion.schemas.actions import LogCreate from prefect.states import ( AwaitingRetry, @@ -414,7 +415,7 @@ async def test_when_num_logs_greater_than_page_size_with_head_then_pagination( expected_line_count=self.PAGE_SIZE + 1, ) - async def test_when_head_without_num_lines_then_return_50_lines( + async def test_when_head_without_num_lines_then_return_the_default_number_of_lines( self, flow_run_factory ): # Given @@ -432,9 +433,9 @@ async def test_when_head_without_num_lines_then_return_50_lines( expected_code=0, expected_output_contains=[ f"Flow run '{flow_run.name}' - Log {i} from flow_run {flow_run.id}." - for i in range(50) + for i in range(LOGS_HEAD_DEFAULT_NUM_LINES) ], - expected_line_count=50, + expected_line_count=LOGS_HEAD_DEFAULT_NUM_LINES, ) async def test_h_and_n_shortcuts_for_head_and_num_lines(self, flow_run_factory): From c0e0bf249c69df0fede0a2412a0b434e82fb0990 Mon Sep 17 00:00:00 2001 From: ohadchaet Date: Wed, 4 Jan 2023 16:30:47 +0200 Subject: [PATCH 08/18] fix: Limits num lines if -n is passed without --head --- src/prefect/cli/flow_run.py | 13 ++++++++----- tests/cli/test_flow_run.py | 6 +++--- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/prefect/cli/flow_run.py b/src/prefect/cli/flow_run.py index 6d52c9e0c4ca..89ca509f4b3f 100644 --- a/src/prefect/cli/flow_run.py +++ b/src/prefect/cli/flow_run.py @@ -148,26 +148,29 @@ async def logs( False, "--head", "-h", - help=f"Show the first {LOGS_HEAD_DEFAULT_NUM_LINES} lines of logs instead of the last 10 lines", + help=f"Show the first {LOGS_HEAD_DEFAULT_NUM_LINES} lines of logs instead of all logs", ), num_lines: int = typer.Option( - LOGS_HEAD_DEFAULT_NUM_LINES, + None, "--num-lines", "-n", - help=f"Number of lines to show when using the --head flag", + help=f"Number of lines to show when using the --head flag. If None, defaults to {LOGS_HEAD_DEFAULT_NUM_LINES}", min=1, ), ): """ View logs for a flow run. """ - # Pagination - API returns max 200 lines at a time + # Pagination - API returns max 200 (LOGS_PAGE_SIZE) lines at a time offset = 0 more_logs = True num_logs_returned = 0 # If head is specified, we need to stop after we've retrieved enough lines - user_specified_num_logs = num_lines if head else None + if head or num_lines: + user_specified_num_logs = num_lines or LOGS_HEAD_DEFAULT_NUM_LINES + else: + user_specified_num_logs = None log_filter = LogFilter(flow_run_id={"any_": [id]}) diff --git a/tests/cli/test_flow_run.py b/tests/cli/test_flow_run.py index 1bf6ccd44d1c..56a4d88f7a77 100644 --- a/tests/cli/test_flow_run.py +++ b/tests/cli/test_flow_run.py @@ -461,7 +461,7 @@ async def test_h_and_n_shortcuts_for_head_and_num_lines(self, flow_run_factory): expected_line_count=10, ) - async def test_when_num_lines_passed_without_head_then_ignore_num_lines( + async def test_when_num_lines_passed_without_head_then_return_n_num_lines( self, flow_run_factory ): # Given @@ -480,9 +480,9 @@ async def test_when_num_lines_passed_without_head_then_ignore_num_lines( expected_code=0, expected_output_contains=[ f"Flow run '{flow_run.name}' - Log {i} from flow_run {flow_run.id}." - for i in range(self.PAGE_SIZE + 1) + for i in range(10) ], - expected_line_count=self.PAGE_SIZE + 1, + expected_line_count=10, ) async def test_when_num_lines_is_smaller_than_one_then_exit_with_error( From b871e1b1b27ce99405647696c3c9f4b2c727e166 Mon Sep 17 00:00:00 2001 From: Ohad Chaet Date: Thu, 5 Jan 2023 21:26:17 +0200 Subject: [PATCH 09/18] Update src/prefect/cli/flow_run.py Co-authored-by: Serina Grill <42048900+serinamarie@users.noreply.github.com> --- src/prefect/cli/flow_run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/prefect/cli/flow_run.py b/src/prefect/cli/flow_run.py index 89ca509f4b3f..d8e54c8740fe 100644 --- a/src/prefect/cli/flow_run.py +++ b/src/prefect/cli/flow_run.py @@ -154,7 +154,7 @@ async def logs( None, "--num-lines", "-n", - help=f"Number of lines to show when using the --head flag. If None, defaults to {LOGS_HEAD_DEFAULT_NUM_LINES}", + help=f"Number of lines to show when using the --head flag. If None, defaults to {LOGS_HEAD_DEFAULT_NUM_LINES}.", min=1, ), ): From b37e90b354c29250d0bce85df6df367d823a410a Mon Sep 17 00:00:00 2001 From: Ohad Chaet Date: Thu, 5 Jan 2023 21:26:34 +0200 Subject: [PATCH 10/18] Update src/prefect/cli/flow_run.py Co-authored-by: Serina Grill <42048900+serinamarie@users.noreply.github.com> --- src/prefect/cli/flow_run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/prefect/cli/flow_run.py b/src/prefect/cli/flow_run.py index d8e54c8740fe..89a8496c5d10 100644 --- a/src/prefect/cli/flow_run.py +++ b/src/prefect/cli/flow_run.py @@ -148,7 +148,7 @@ async def logs( False, "--head", "-h", - help=f"Show the first {LOGS_HEAD_DEFAULT_NUM_LINES} lines of logs instead of all logs", + help=f"Show the first {LOGS_HEAD_DEFAULT_NUM_LINES} lines of logs instead of all logs.", ), num_lines: int = typer.Option( None, From 2edf922795ef27a2421a35eb5edfa47f7a3d629c Mon Sep 17 00:00:00 2001 From: ohadchaet Date: Thu, 5 Jan 2023 21:28:11 +0200 Subject: [PATCH 11/18] refactor: renames constants to follow the same convention --- src/prefect/cli/flow_run.py | 20 ++++++++++---------- tests/cli/test_flow_run.py | 6 +++--- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/prefect/cli/flow_run.py b/src/prefect/cli/flow_run.py index 89a8496c5d10..d6ef1440d256 100644 --- a/src/prefect/cli/flow_run.py +++ b/src/prefect/cli/flow_run.py @@ -28,8 +28,8 @@ ) app.add_typer(flow_run_app, aliases=["flow-runs"]) -LOGS_PAGE_SIZE = 200 -LOGS_HEAD_DEFAULT_NUM_LINES = 20 +LOGS_DEFAULT_PAGE_SIZE = 200 +LOGS_DEFAULT_HEAD_NUM_LINES = 20 @flow_run_app.command() @@ -148,27 +148,27 @@ async def logs( False, "--head", "-h", - help=f"Show the first {LOGS_HEAD_DEFAULT_NUM_LINES} lines of logs instead of all logs.", + help=f"Show the first {LOGS_DEFAULT_HEAD_NUM_LINES} lines of logs instead of all logs.", ), num_lines: int = typer.Option( None, "--num-lines", "-n", - help=f"Number of lines to show when using the --head flag. If None, defaults to {LOGS_HEAD_DEFAULT_NUM_LINES}.", + help=f"Number of lines to show when using the --head flag. If None, defaults to {LOGS_DEFAULT_HEAD_NUM_LINES}.", min=1, ), ): """ View logs for a flow run. """ - # Pagination - API returns max 200 (LOGS_PAGE_SIZE) lines at a time + # Pagination - API returns max 200 (LOGS_DEFAULT_PAGE_SIZE) lines at a time offset = 0 more_logs = True num_logs_returned = 0 # If head is specified, we need to stop after we've retrieved enough lines if head or num_lines: - user_specified_num_logs = num_lines or LOGS_HEAD_DEFAULT_NUM_LINES + user_specified_num_logs = num_lines or LOGS_DEFAULT_HEAD_NUM_LINES else: user_specified_num_logs = None @@ -183,9 +183,9 @@ async def logs( while more_logs: num_logs_to_return_from_page = ( - LOGS_PAGE_SIZE + LOGS_DEFAULT_PAGE_SIZE if user_specified_num_logs is None - else min(LOGS_PAGE_SIZE, user_specified_num_logs) + else min(LOGS_DEFAULT_PAGE_SIZE, user_specified_num_logs) ) # Get the next page of logs @@ -204,8 +204,8 @@ async def logs( # Update the number of logs retrieved num_logs_returned += num_logs_to_return_from_page - if len(page_logs) == LOGS_PAGE_SIZE: - offset += LOGS_PAGE_SIZE + if len(page_logs) == LOGS_DEFAULT_PAGE_SIZE: + offset += LOGS_DEFAULT_PAGE_SIZE else: # No more logs to show, exit more_logs = False diff --git a/tests/cli/test_flow_run.py b/tests/cli/test_flow_run.py index 56a4d88f7a77..466925e00c61 100644 --- a/tests/cli/test_flow_run.py +++ b/tests/cli/test_flow_run.py @@ -5,7 +5,7 @@ import prefect.exceptions from prefect import flow -from prefect.cli.flow_run import LOGS_HEAD_DEFAULT_NUM_LINES +from prefect.cli.flow_run import LOGS_DEFAULT_HEAD_NUM_LINES from prefect.orion.schemas.actions import LogCreate from prefect.states import ( AwaitingRetry, @@ -433,9 +433,9 @@ async def test_when_head_without_num_lines_then_return_the_default_number_of_lin expected_code=0, expected_output_contains=[ f"Flow run '{flow_run.name}' - Log {i} from flow_run {flow_run.id}." - for i in range(LOGS_HEAD_DEFAULT_NUM_LINES) + for i in range(LOGS_DEFAULT_HEAD_NUM_LINES) ], - expected_line_count=LOGS_HEAD_DEFAULT_NUM_LINES, + expected_line_count=LOGS_DEFAULT_HEAD_NUM_LINES, ) async def test_h_and_n_shortcuts_for_head_and_num_lines(self, flow_run_factory): From 6f66e0c9943f8ce4db0d30deb58bdcea511bc79e Mon Sep 17 00:00:00 2001 From: Ohad Chaet Date: Thu, 5 Jan 2023 21:29:02 +0200 Subject: [PATCH 12/18] Update tests/cli/test_flow_run.py Co-authored-by: Serina Grill <42048900+serinamarie@users.noreply.github.com> --- tests/cli/test_flow_run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cli/test_flow_run.py b/tests/cli/test_flow_run.py index 466925e00c61..4dc374ce40c4 100644 --- a/tests/cli/test_flow_run.py +++ b/tests/cli/test_flow_run.py @@ -415,7 +415,7 @@ async def test_when_num_logs_greater_than_page_size_with_head_then_pagination( expected_line_count=self.PAGE_SIZE + 1, ) - async def test_when_head_without_num_lines_then_return_the_default_number_of_lines( + async def test_default_head_returns_default_num_logs( self, flow_run_factory ): # Given From 6109acf7760136f05a3a40926dfff67386d8764e Mon Sep 17 00:00:00 2001 From: Ohad Chaet Date: Thu, 5 Jan 2023 21:29:10 +0200 Subject: [PATCH 13/18] Update tests/cli/test_flow_run.py Co-authored-by: Serina Grill <42048900+serinamarie@users.noreply.github.com> --- tests/cli/test_flow_run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cli/test_flow_run.py b/tests/cli/test_flow_run.py index 4dc374ce40c4..d680b058051a 100644 --- a/tests/cli/test_flow_run.py +++ b/tests/cli/test_flow_run.py @@ -461,7 +461,7 @@ async def test_h_and_n_shortcuts_for_head_and_num_lines(self, flow_run_factory): expected_line_count=10, ) - async def test_when_num_lines_passed_without_head_then_return_n_num_lines( + async def test_num_logs_passed_standalone_returns_num_logs( self, flow_run_factory ): # Given From 045a8ab2d44bcae2bb09dc28d84bb1caf9a8d40b Mon Sep 17 00:00:00 2001 From: ohadchaet Date: Thu, 5 Jan 2023 21:30:47 +0200 Subject: [PATCH 14/18] refactor: renames test constant to be clearer --- tests/cli/test_flow_run.py | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/tests/cli/test_flow_run.py b/tests/cli/test_flow_run.py index d680b058051a..868dcbadb43d 100644 --- a/tests/cli/test_flow_run.py +++ b/tests/cli/test_flow_run.py @@ -305,13 +305,13 @@ async def create_flow_run(num_logs: int): class TestFlowRunLogs: - PAGE_SIZE = 200 + LOGS_DEFAULT_PAGE_SIZE = 200 async def test_when_num_logs_smaller_than_page_size_then_no_pagination( self, flow_run_factory ): # Given - flow_run = await flow_run_factory(num_logs=self.PAGE_SIZE - 1) + flow_run = await flow_run_factory(num_logs=self.LOGS_DEFAULT_PAGE_SIZE - 1) # When/Then await run_sync_in_worker_thread( @@ -324,7 +324,7 @@ async def test_when_num_logs_smaller_than_page_size_then_no_pagination( expected_code=0, expected_output_contains=[ f"Flow run '{flow_run.name}' - Log {i} from flow_run {flow_run.id}." - for i in range(self.PAGE_SIZE - 1) + for i in range(self.LOGS_DEFAULT_PAGE_SIZE - 1) ], ) @@ -332,7 +332,7 @@ async def test_when_num_logs_greater_than_page_size_then_pagination( self, flow_run_factory ): # Given - flow_run = await flow_run_factory(num_logs=self.PAGE_SIZE + 1) + flow_run = await flow_run_factory(num_logs=self.LOGS_DEFAULT_PAGE_SIZE + 1) # When/Then await run_sync_in_worker_thread( @@ -345,7 +345,7 @@ async def test_when_num_logs_greater_than_page_size_then_pagination( expected_code=0, expected_output_contains=[ f"Flow run '{flow_run.name}' - Log {i} from flow_run {flow_run.id}." - for i in range(self.PAGE_SIZE + 1) + for i in range(self.LOGS_DEFAULT_PAGE_SIZE + 1) ], ) @@ -369,7 +369,7 @@ async def test_when_num_logs_smaller_than_page_size_with_head_then_no_pagination self, flow_run_factory ): # Given - flow_run = await flow_run_factory(num_logs=self.PAGE_SIZE + 1) + flow_run = await flow_run_factory(num_logs=self.LOGS_DEFAULT_PAGE_SIZE + 1) # When/Then await run_sync_in_worker_thread( @@ -394,7 +394,7 @@ async def test_when_num_logs_greater_than_page_size_with_head_then_pagination( self, flow_run_factory ): # Given - flow_run = await flow_run_factory(num_logs=self.PAGE_SIZE + 1) + flow_run = await flow_run_factory(num_logs=self.LOGS_DEFAULT_PAGE_SIZE + 1) # When/Then await run_sync_in_worker_thread( @@ -405,21 +405,19 @@ async def test_when_num_logs_greater_than_page_size_with_head_then_pagination( str(flow_run.id), "--head", "--num-lines", - self.PAGE_SIZE + 1, + self.LOGS_DEFAULT_PAGE_SIZE + 1, ], expected_code=0, expected_output_contains=[ f"Flow run '{flow_run.name}' - Log {i} from flow_run {flow_run.id}." - for i in range(self.PAGE_SIZE + 1) + for i in range(self.LOGS_DEFAULT_PAGE_SIZE + 1) ], - expected_line_count=self.PAGE_SIZE + 1, + expected_line_count=self.LOGS_DEFAULT_PAGE_SIZE + 1, ) - async def test_default_head_returns_default_num_logs( - self, flow_run_factory - ): + async def test_default_head_returns_default_num_logs(self, flow_run_factory): # Given - flow_run = await flow_run_factory(num_logs=self.PAGE_SIZE + 1) + flow_run = await flow_run_factory(num_logs=self.LOGS_DEFAULT_PAGE_SIZE + 1) # When/Then await run_sync_in_worker_thread( @@ -440,7 +438,7 @@ async def test_default_head_returns_default_num_logs( async def test_h_and_n_shortcuts_for_head_and_num_lines(self, flow_run_factory): # Given - flow_run = await flow_run_factory(num_logs=self.PAGE_SIZE + 1) + flow_run = await flow_run_factory(num_logs=self.LOGS_DEFAULT_PAGE_SIZE + 1) # When/Then await run_sync_in_worker_thread( @@ -461,11 +459,9 @@ async def test_h_and_n_shortcuts_for_head_and_num_lines(self, flow_run_factory): expected_line_count=10, ) - async def test_num_logs_passed_standalone_returns_num_logs( - self, flow_run_factory - ): + async def test_num_logs_passed_standalone_returns_num_logs(self, flow_run_factory): # Given - flow_run = await flow_run_factory(num_logs=self.PAGE_SIZE + 1) + flow_run = await flow_run_factory(num_logs=self.LOGS_DEFAULT_PAGE_SIZE + 1) # When/Then await run_sync_in_worker_thread( @@ -489,7 +485,7 @@ async def test_when_num_lines_is_smaller_than_one_then_exit_with_error( self, flow_run_factory ): # Given - flow_run = await flow_run_factory(num_logs=self.PAGE_SIZE + 1) + flow_run = await flow_run_factory(num_logs=self.LOGS_DEFAULT_PAGE_SIZE + 1) # When/Then await run_sync_in_worker_thread( From eb45465cff994490bc9d2dc1f5fb90e3f4ffafbd Mon Sep 17 00:00:00 2001 From: ohadchaet Date: Thu, 5 Jan 2023 21:33:04 +0200 Subject: [PATCH 15/18] refactor: renames num-lines to num-logs --- src/prefect/cli/flow_run.py | 2 +- tests/cli/test_flow_run.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/prefect/cli/flow_run.py b/src/prefect/cli/flow_run.py index d6ef1440d256..ff5c2e5ab868 100644 --- a/src/prefect/cli/flow_run.py +++ b/src/prefect/cli/flow_run.py @@ -152,7 +152,7 @@ async def logs( ), num_lines: int = typer.Option( None, - "--num-lines", + "--num-logs", "-n", help=f"Number of lines to show when using the --head flag. If None, defaults to {LOGS_DEFAULT_HEAD_NUM_LINES}.", min=1, diff --git a/tests/cli/test_flow_run.py b/tests/cli/test_flow_run.py index 868dcbadb43d..2f957f0f41cd 100644 --- a/tests/cli/test_flow_run.py +++ b/tests/cli/test_flow_run.py @@ -379,7 +379,7 @@ async def test_when_num_logs_smaller_than_page_size_with_head_then_no_pagination "logs", str(flow_run.id), "--head", - "--num-lines", + "--num-logs", "10", ], expected_code=0, @@ -404,7 +404,7 @@ async def test_when_num_logs_greater_than_page_size_with_head_then_pagination( "logs", str(flow_run.id), "--head", - "--num-lines", + "--num-logs", self.LOGS_DEFAULT_PAGE_SIZE + 1, ], expected_code=0, @@ -470,7 +470,7 @@ async def test_num_logs_passed_standalone_returns_num_logs(self, flow_run_factor "flow-run", "logs", str(flow_run.id), - "--num-lines", + "--num-logs", "10", ], expected_code=0, @@ -494,9 +494,9 @@ async def test_when_num_lines_is_smaller_than_one_then_exit_with_error( "flow-run", "logs", str(flow_run.id), - "--num-lines", + "--num-logs", "0", ], expected_code=2, - expected_output_contains="Invalid value for '--num-lines' / '-n': 0 is not in the range x>=1.", + expected_output_contains="Invalid value for '--num-logs' / '-n': 0 is not in the range x>=1.", ) From c57e7c833ac2456c4b8307fa93e7b9dc2d26bfb2 Mon Sep 17 00:00:00 2001 From: ohadchaet Date: Fri, 6 Jan 2023 08:15:13 +0200 Subject: [PATCH 16/18] refactor: better naming of constant --- src/prefect/cli/flow_run.py | 8 ++++---- tests/cli/test_flow_run.py | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/prefect/cli/flow_run.py b/src/prefect/cli/flow_run.py index ff5c2e5ab868..f4f24071216c 100644 --- a/src/prefect/cli/flow_run.py +++ b/src/prefect/cli/flow_run.py @@ -29,7 +29,7 @@ app.add_typer(flow_run_app, aliases=["flow-runs"]) LOGS_DEFAULT_PAGE_SIZE = 200 -LOGS_DEFAULT_HEAD_NUM_LINES = 20 +LOGS_WITH_LIMIT_FLAG_DEFAULT_NUM_LOGS = 20 @flow_run_app.command() @@ -148,13 +148,13 @@ async def logs( False, "--head", "-h", - help=f"Show the first {LOGS_DEFAULT_HEAD_NUM_LINES} lines of logs instead of all logs.", + help=f"Show the first {LOGS_WITH_LIMIT_FLAG_DEFAULT_NUM_LOGS} lines of logs instead of all logs.", ), num_lines: int = typer.Option( None, "--num-logs", "-n", - help=f"Number of lines to show when using the --head flag. If None, defaults to {LOGS_DEFAULT_HEAD_NUM_LINES}.", + help=f"Number of lines to show when using the --head flag. If None, defaults to {LOGS_WITH_LIMIT_FLAG_DEFAULT_NUM_LOGS}.", min=1, ), ): @@ -168,7 +168,7 @@ async def logs( # If head is specified, we need to stop after we've retrieved enough lines if head or num_lines: - user_specified_num_logs = num_lines or LOGS_DEFAULT_HEAD_NUM_LINES + user_specified_num_logs = num_lines or LOGS_WITH_LIMIT_FLAG_DEFAULT_NUM_LOGS else: user_specified_num_logs = None diff --git a/tests/cli/test_flow_run.py b/tests/cli/test_flow_run.py index 2f957f0f41cd..e82354f7a477 100644 --- a/tests/cli/test_flow_run.py +++ b/tests/cli/test_flow_run.py @@ -5,7 +5,7 @@ import prefect.exceptions from prefect import flow -from prefect.cli.flow_run import LOGS_DEFAULT_HEAD_NUM_LINES +from prefect.cli.flow_run import LOGS_WITH_LIMIT_FLAG_DEFAULT_NUM_LOGS from prefect.orion.schemas.actions import LogCreate from prefect.states import ( AwaitingRetry, @@ -431,9 +431,9 @@ async def test_default_head_returns_default_num_logs(self, flow_run_factory): expected_code=0, expected_output_contains=[ f"Flow run '{flow_run.name}' - Log {i} from flow_run {flow_run.id}." - for i in range(LOGS_DEFAULT_HEAD_NUM_LINES) + for i in range(LOGS_WITH_LIMIT_FLAG_DEFAULT_NUM_LOGS) ], - expected_line_count=LOGS_DEFAULT_HEAD_NUM_LINES, + expected_line_count=LOGS_WITH_LIMIT_FLAG_DEFAULT_NUM_LOGS, ) async def test_h_and_n_shortcuts_for_head_and_num_lines(self, flow_run_factory): From ca18c51df38c865624c2d987f7268fb8b722c651 Mon Sep 17 00:00:00 2001 From: ohadchaet Date: Fri, 6 Jan 2023 18:49:36 +0200 Subject: [PATCH 17/18] fix: renames occurrences of 'lines' to 'logs' --- src/prefect/cli/flow_run.py | 14 +++++++------- tests/cli/test_flow_run.py | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/prefect/cli/flow_run.py b/src/prefect/cli/flow_run.py index f4f24071216c..86ac56cf22cd 100644 --- a/src/prefect/cli/flow_run.py +++ b/src/prefect/cli/flow_run.py @@ -148,27 +148,27 @@ async def logs( False, "--head", "-h", - help=f"Show the first {LOGS_WITH_LIMIT_FLAG_DEFAULT_NUM_LOGS} lines of logs instead of all logs.", + help=f"Show the first {LOGS_WITH_LIMIT_FLAG_DEFAULT_NUM_LOGS} logs instead of all logs.", ), - num_lines: int = typer.Option( + num_logs: int = typer.Option( None, "--num-logs", "-n", - help=f"Number of lines to show when using the --head flag. If None, defaults to {LOGS_WITH_LIMIT_FLAG_DEFAULT_NUM_LOGS}.", + help=f"Number of logs to show when using the --head flag. If None, defaults to {LOGS_WITH_LIMIT_FLAG_DEFAULT_NUM_LOGS}.", min=1, ), ): """ View logs for a flow run. """ - # Pagination - API returns max 200 (LOGS_DEFAULT_PAGE_SIZE) lines at a time + # Pagination - API returns max 200 (LOGS_DEFAULT_PAGE_SIZE) logs at a time offset = 0 more_logs = True num_logs_returned = 0 - # If head is specified, we need to stop after we've retrieved enough lines - if head or num_lines: - user_specified_num_logs = num_lines or LOGS_WITH_LIMIT_FLAG_DEFAULT_NUM_LOGS + # If head is specified, we need to stop after we've retrieved enough logs + if head or num_logs: + user_specified_num_logs = num_logs or LOGS_WITH_LIMIT_FLAG_DEFAULT_NUM_LOGS else: user_specified_num_logs = None diff --git a/tests/cli/test_flow_run.py b/tests/cli/test_flow_run.py index e82354f7a477..7940987cb4a1 100644 --- a/tests/cli/test_flow_run.py +++ b/tests/cli/test_flow_run.py @@ -436,7 +436,7 @@ async def test_default_head_returns_default_num_logs(self, flow_run_factory): expected_line_count=LOGS_WITH_LIMIT_FLAG_DEFAULT_NUM_LOGS, ) - async def test_h_and_n_shortcuts_for_head_and_num_lines(self, flow_run_factory): + async def test_h_and_n_shortcuts_for_head_and_num_logs(self, flow_run_factory): # Given flow_run = await flow_run_factory(num_logs=self.LOGS_DEFAULT_PAGE_SIZE + 1) @@ -481,7 +481,7 @@ async def test_num_logs_passed_standalone_returns_num_logs(self, flow_run_factor expected_line_count=10, ) - async def test_when_num_lines_is_smaller_than_one_then_exit_with_error( + async def test_when_num_logs_is_smaller_than_one_then_exit_with_error( self, flow_run_factory ): # Given From cfdbc0464392c698f90c0f22e2eb60279ff9a6f6 Mon Sep 17 00:00:00 2001 From: Ohad Chaet Date: Tue, 17 Jan 2023 18:04:24 +0200 Subject: [PATCH 18/18] Update tests/cli/test_flow_run.py Co-authored-by: Serina Grill <42048900+serinamarie@users.noreply.github.com> --- tests/cli/test_flow_run.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/cli/test_flow_run.py b/tests/cli/test_flow_run.py index 7940987cb4a1..a091bb18fd78 100644 --- a/tests/cli/test_flow_run.py +++ b/tests/cli/test_flow_run.py @@ -481,6 +481,7 @@ async def test_num_logs_passed_standalone_returns_num_logs(self, flow_run_factor expected_line_count=10, ) + @pytest.mark.skip(reason="we need to disable colors for this test to pass") async def test_when_num_logs_is_smaller_than_one_then_exit_with_error( self, flow_run_factory ):