From 21ad4e0f7ba49eab4f3696bdac30b78c87af81fb Mon Sep 17 00:00:00 2001 From: Alex Streed Date: Thu, 12 Dec 2024 10:48:43 -0600 Subject: [PATCH 1/9] Improve guidance when type completeness check fails --- .github/workflows/static-analysis.yaml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/workflows/static-analysis.yaml b/.github/workflows/static-analysis.yaml index c9f9ff18409d..8567660cb754 100644 --- a/.github/workflows/static-analysis.yaml +++ b/.github/workflows/static-analysis.yaml @@ -108,12 +108,16 @@ jobs: BASE_SCORE=$(echo ${{ steps.calculate_base_score.outputs.base_score }}) if (( $(echo "$BASE_SCORE > $CURRENT_SCORE" | bc -l) )); then - echo "❌ Type completeness score has decreased from $BASE_SCORE to $CURRENT_SCORE" >> $GITHUB_STEP_SUMMARY - echo "Please add type annotations to your code to increase the type completeness score." >> $GITHUB_STEP_SUMMARY + echo "::notice We noticed a decrease in type coverage with these changes. Check workflow summary for more details." + echo "ℹī¸ Type Completeness Check" >> $GITHUB_STEP_SUMMARY + echo "We noticed a decrease in type coverage with these changes." >> $GITHUB_STEP_SUMMARY + echo "To maintain our codebase quality, we aim to keep or improve type coverage with each change." >> $GITHUB_STEP_SUMMARY + echo "Here's what changed:" >> $GITHUB_STEP_SUMMARY uv run scripts/pyright_diff.py prefect-analysis-base.json prefect-analysis.json >> $GITHUB_STEP_SUMMARY + echo "Need help? Ping @desertaxle or @zzstoatzz for assistance!" >> $GITHUB_STEP_SUMMARY exit 1 elif (( $(echo "$BASE_SCORE < $CURRENT_SCORE" | bc -l) )); then - echo "✅ Type completeness score has increased from $BASE_SCORE to $CURRENT_SCORE" >> $GITHUB_STEP_SUMMARY + echo "🎉 Great work! The type coverage has improved with these changes" >> $GITHUB_STEP_SUMMARY else - echo "✅ Type completeness score remained unchanged at $BASE_SCORE" >> $GITHUB_STEP_SUMMARY + echo "✅ Type coverage maintained" >> $GITHUB_STEP_SUMMARY fi From 831144fe134c03d6cbc79221f52be3557d4ab815 Mon Sep 17 00:00:00 2001 From: Alex Streed Date: Thu, 12 Dec 2024 11:03:31 -0600 Subject: [PATCH 2/9] Add buffer to type completeness check --- .github/workflows/static-analysis.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/static-analysis.yaml b/.github/workflows/static-analysis.yaml index 8567660cb754..038b296c2e66 100644 --- a/.github/workflows/static-analysis.yaml +++ b/.github/workflows/static-analysis.yaml @@ -115,7 +115,10 @@ jobs: echo "Here's what changed:" >> $GITHUB_STEP_SUMMARY uv run scripts/pyright_diff.py prefect-analysis-base.json prefect-analysis.json >> $GITHUB_STEP_SUMMARY echo "Need help? Ping @desertaxle or @zzstoatzz for assistance!" >> $GITHUB_STEP_SUMMARY - exit 1 + SCORE_DIFF=$(echo "$BASE_SCORE - $CURRENT_SCORE" | bc -l) + if (( $(echo "$SCORE_DIFF > 0.001" | bc -l) )); then + exit 1 + fi elif (( $(echo "$BASE_SCORE < $CURRENT_SCORE" | bc -l) )); then echo "🎉 Great work! The type coverage has improved with these changes" >> $GITHUB_STEP_SUMMARY else From 0f12c1a677265232070c86ec025e68c1f64d6115 Mon Sep 17 00:00:00 2001 From: Alex Streed Date: Thu, 12 Dec 2024 11:05:29 -0600 Subject: [PATCH 3/9] Introduce some type errors --- src/prefect/logging/loggers.py | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/prefect/logging/loggers.py b/src/prefect/logging/loggers.py index 4a9211a8d95b..d9a7cb2de798 100644 --- a/src/prefect/logging/loggers.py +++ b/src/prefect/logging/loggers.py @@ -14,11 +14,7 @@ from prefect.telemetry.logging import add_telemetry_log_handler if TYPE_CHECKING: - from prefect.client.schemas import FlowRun as ClientFlowRun - from prefect.client.schemas.objects import FlowRun, TaskRun from prefect.context import RunContext - from prefect.flows import Flow - from prefect.tasks import Task from prefect.workers.base import BaseWorker if sys.version_info >= (3, 12): @@ -87,7 +83,7 @@ def get_logger(name: Optional[str] = None) -> logging.Logger: def get_run_logger( - context: Optional["RunContext"] = None, **kwargs: Any + context: Optional["RunContext"] = None, **kwargs ) -> Union[logging.Logger, LoggingAdapter]: """ Get a Prefect logger for the current task run or flow run. @@ -158,9 +154,9 @@ def get_run_logger( def flow_run_logger( - flow_run: Union["FlowRun", "ClientFlowRun"], - flow: Optional["Flow[Any, Any]"] = None, - **kwargs: str, + flow_run, + flow=None, + **kwargs, ) -> LoggingAdapter: """ Create a flow run logger with the run's metadata attached. @@ -184,11 +180,11 @@ def flow_run_logger( def task_run_logger( - task_run: "TaskRun", - task: Optional["Task[Any, Any]"] = None, - flow_run: Optional["FlowRun"] = None, - flow: Optional["Flow[Any, Any]"] = None, - **kwargs: Any, + task_run, + task=None, + flow_run=None, + flow=None, + **kwargs, ): """ Create a task run logger with the run's metadata attached. From 351716f51edc791aa6ac7976fa1b42d0a93be525 Mon Sep 17 00:00:00 2001 From: Alex Streed Date: Thu, 12 Dec 2024 11:08:03 -0600 Subject: [PATCH 4/9] More type errors --- src/prefect/logging/loggers.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/prefect/logging/loggers.py b/src/prefect/logging/loggers.py index d9a7cb2de798..872a259c6ba2 100644 --- a/src/prefect/logging/loggers.py +++ b/src/prefect/logging/loggers.py @@ -5,7 +5,7 @@ from contextlib import contextmanager from functools import lru_cache from logging import LogRecord -from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union +from typing import TYPE_CHECKING, Any, Dict, List, Optional from typing_extensions import Self @@ -14,8 +14,7 @@ from prefect.telemetry.logging import add_telemetry_log_handler if TYPE_CHECKING: - from prefect.context import RunContext - from prefect.workers.base import BaseWorker + pass if sys.version_info >= (3, 12): LoggingAdapter = logging.LoggerAdapter[logging.Logger] @@ -53,7 +52,7 @@ def getChild( @lru_cache() -def get_logger(name: Optional[str] = None) -> logging.Logger: +def get_logger(name=None) -> logging.Logger: """ Get a `prefect` logger. These loggers are intended for internal use within the `prefect` package. @@ -82,9 +81,7 @@ def get_logger(name: Optional[str] = None) -> logging.Logger: return logger -def get_run_logger( - context: Optional["RunContext"] = None, **kwargs -) -> Union[logging.Logger, LoggingAdapter]: +def get_run_logger(context=None, **kwargs): """ Get a Prefect logger for the current task run or flow run. @@ -221,7 +218,7 @@ def task_run_logger( ) -def get_worker_logger(worker: "BaseWorker", name: Optional[str] = None): +def get_worker_logger(worker, name=None): """ Create a worker logger with the worker's metadata attached. @@ -245,7 +242,7 @@ def get_worker_logger(worker: "BaseWorker", name: Optional[str] = None): @contextmanager -def disable_logger(name: str): +def disable_logger(name): """ Get a logger by name and disables it within the context manager. Upon exiting the context manager, the logger is returned to its From ae050b8cba49d1d395353ac54c092e6e02d0ade1 Mon Sep 17 00:00:00 2001 From: Alex Streed Date: Thu, 12 Dec 2024 11:17:15 -0600 Subject: [PATCH 5/9] Introduce a big error --- src/prefect/_internal/schemas/bases.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/prefect/_internal/schemas/bases.py b/src/prefect/_internal/schemas/bases.py index 62804f8b478a..fe17417a3388 100644 --- a/src/prefect/_internal/schemas/bases.py +++ b/src/prefect/_internal/schemas/bases.py @@ -34,7 +34,7 @@ class PrefectBaseModel(BaseModel): _reset_fields: ClassVar[Set[str]] = set() - model_config: ClassVar[ConfigDict] = ConfigDict( + model_config = ConfigDict( ser_json_timedelta="float", defer_build=True, extra=( From f9caa89b0f98d308570ecd232f109c89853082e4 Mon Sep 17 00:00:00 2001 From: Alex Streed Date: Thu, 12 Dec 2024 11:21:40 -0600 Subject: [PATCH 6/9] Try different formatting --- .github/workflows/static-analysis.yaml | 7 +++---- scripts/pyright_diff.py | 1 - 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/.github/workflows/static-analysis.yaml b/.github/workflows/static-analysis.yaml index 038b296c2e66..15747d02060f 100644 --- a/.github/workflows/static-analysis.yaml +++ b/.github/workflows/static-analysis.yaml @@ -109,12 +109,11 @@ jobs: if (( $(echo "$BASE_SCORE > $CURRENT_SCORE" | bc -l) )); then echo "::notice We noticed a decrease in type coverage with these changes. Check workflow summary for more details." - echo "ℹī¸ Type Completeness Check" >> $GITHUB_STEP_SUMMARY - echo "We noticed a decrease in type coverage with these changes." >> $GITHUB_STEP_SUMMARY - echo "To maintain our codebase quality, we aim to keep or improve type coverage with each change." >> $GITHUB_STEP_SUMMARY + echo "### ℹī¸ Type Completeness Check" >> $GITHUB_STEP_SUMMARY + echo "We noticed a decrease in type coverage with these changes. To maintain our codebase quality, we aim to keep or improve type coverage with each change." >> $GITHUB_STEP_SUMMARY + echo "Need help? Ping @desertaxle or @zzstoatzz for assistance!" >> $GITHUB_STEP_SUMMARY echo "Here's what changed:" >> $GITHUB_STEP_SUMMARY uv run scripts/pyright_diff.py prefect-analysis-base.json prefect-analysis.json >> $GITHUB_STEP_SUMMARY - echo "Need help? Ping @desertaxle or @zzstoatzz for assistance!" >> $GITHUB_STEP_SUMMARY SCORE_DIFF=$(echo "$BASE_SCORE - $CURRENT_SCORE" | bc -l) if (( $(echo "$SCORE_DIFF > 0.001" | bc -l) )); then exit 1 diff --git a/scripts/pyright_diff.py b/scripts/pyright_diff.py index ceaf43a1601b..17dee6f93172 100644 --- a/scripts/pyright_diff.py +++ b/scripts/pyright_diff.py @@ -76,7 +76,6 @@ def compare_pyright_outputs(base_file: str, new_file: str) -> None: # Find new errors new_errors = list(new_diags - base_diags) - print("\n## New Pyright Errors\n") print(format_markdown_table(new_errors)) From 08b91923fb710030807eaa4c7bc54d73704f1c1c Mon Sep 17 00:00:00 2001 From: Alex Streed Date: Thu, 12 Dec 2024 11:25:37 -0600 Subject: [PATCH 7/9] Try to get message to work --- .github/workflows/static-analysis.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/static-analysis.yaml b/.github/workflows/static-analysis.yaml index 15747d02060f..603b8a1d01d4 100644 --- a/.github/workflows/static-analysis.yaml +++ b/.github/workflows/static-analysis.yaml @@ -108,11 +108,11 @@ jobs: BASE_SCORE=$(echo ${{ steps.calculate_base_score.outputs.base_score }}) if (( $(echo "$BASE_SCORE > $CURRENT_SCORE" | bc -l) )); then - echo "::notice We noticed a decrease in type coverage with these changes. Check workflow summary for more details." + echo "::notice title=Type Completeness Check::We noticed a decrease in type coverage with these changes. Check workflow summary for more details." echo "### ℹī¸ Type Completeness Check" >> $GITHUB_STEP_SUMMARY echo "We noticed a decrease in type coverage with these changes. To maintain our codebase quality, we aim to keep or improve type coverage with each change." >> $GITHUB_STEP_SUMMARY - echo "Need help? Ping @desertaxle or @zzstoatzz for assistance!" >> $GITHUB_STEP_SUMMARY - echo "Here's what changed:" >> $GITHUB_STEP_SUMMARY + echo "\nNeed help? Ping @desertaxle or @zzstoatzz for assistance!" >> $GITHUB_STEP_SUMMARY + echo "\nHere's what changed:" >> $GITHUB_STEP_SUMMARY uv run scripts/pyright_diff.py prefect-analysis-base.json prefect-analysis.json >> $GITHUB_STEP_SUMMARY SCORE_DIFF=$(echo "$BASE_SCORE - $CURRENT_SCORE" | bc -l) if (( $(echo "$SCORE_DIFF > 0.001" | bc -l) )); then From 599d86a91206a63583a92e538fe0f3f8acf071c7 Mon Sep 17 00:00:00 2001 From: Alex Streed Date: Thu, 12 Dec 2024 11:28:04 -0600 Subject: [PATCH 8/9] newlines don't work --- .github/workflows/static-analysis.yaml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/static-analysis.yaml b/.github/workflows/static-analysis.yaml index 603b8a1d01d4..0bb1c20d485e 100644 --- a/.github/workflows/static-analysis.yaml +++ b/.github/workflows/static-analysis.yaml @@ -111,8 +111,10 @@ jobs: echo "::notice title=Type Completeness Check::We noticed a decrease in type coverage with these changes. Check workflow summary for more details." echo "### ℹī¸ Type Completeness Check" >> $GITHUB_STEP_SUMMARY echo "We noticed a decrease in type coverage with these changes. To maintain our codebase quality, we aim to keep or improve type coverage with each change." >> $GITHUB_STEP_SUMMARY - echo "\nNeed help? Ping @desertaxle or @zzstoatzz for assistance!" >> $GITHUB_STEP_SUMMARY - echo "\nHere's what changed:" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "Need help? Ping @desertaxle or @zzstoatzz for assistance!" >> $GITHUB_STEP_SUMMARY + echo "Here's what changed:" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY uv run scripts/pyright_diff.py prefect-analysis-base.json prefect-analysis.json >> $GITHUB_STEP_SUMMARY SCORE_DIFF=$(echo "$BASE_SCORE - $CURRENT_SCORE" | bc -l) if (( $(echo "$SCORE_DIFF > 0.001" | bc -l) )); then From b31c47bff8a1c8dfda3fd93f47706cecdb27f842 Mon Sep 17 00:00:00 2001 From: Alex Streed Date: Thu, 12 Dec 2024 11:32:43 -0600 Subject: [PATCH 9/9] Revert type errors --- .github/workflows/static-analysis.yaml | 1 + src/prefect/_internal/schemas/bases.py | 2 +- src/prefect/logging/loggers.py | 35 +++++++++++++++----------- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/.github/workflows/static-analysis.yaml b/.github/workflows/static-analysis.yaml index 0bb1c20d485e..96146dec5f89 100644 --- a/.github/workflows/static-analysis.yaml +++ b/.github/workflows/static-analysis.yaml @@ -113,6 +113,7 @@ jobs: echo "We noticed a decrease in type coverage with these changes. To maintain our codebase quality, we aim to keep or improve type coverage with each change." >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY echo "Need help? Ping @desertaxle or @zzstoatzz for assistance!" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY echo "Here's what changed:" >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY uv run scripts/pyright_diff.py prefect-analysis-base.json prefect-analysis.json >> $GITHUB_STEP_SUMMARY diff --git a/src/prefect/_internal/schemas/bases.py b/src/prefect/_internal/schemas/bases.py index fe17417a3388..62804f8b478a 100644 --- a/src/prefect/_internal/schemas/bases.py +++ b/src/prefect/_internal/schemas/bases.py @@ -34,7 +34,7 @@ class PrefectBaseModel(BaseModel): _reset_fields: ClassVar[Set[str]] = set() - model_config = ConfigDict( + model_config: ClassVar[ConfigDict] = ConfigDict( ser_json_timedelta="float", defer_build=True, extra=( diff --git a/src/prefect/logging/loggers.py b/src/prefect/logging/loggers.py index 872a259c6ba2..4a9211a8d95b 100644 --- a/src/prefect/logging/loggers.py +++ b/src/prefect/logging/loggers.py @@ -5,7 +5,7 @@ from contextlib import contextmanager from functools import lru_cache from logging import LogRecord -from typing import TYPE_CHECKING, Any, Dict, List, Optional +from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union from typing_extensions import Self @@ -14,7 +14,12 @@ from prefect.telemetry.logging import add_telemetry_log_handler if TYPE_CHECKING: - pass + from prefect.client.schemas import FlowRun as ClientFlowRun + from prefect.client.schemas.objects import FlowRun, TaskRun + from prefect.context import RunContext + from prefect.flows import Flow + from prefect.tasks import Task + from prefect.workers.base import BaseWorker if sys.version_info >= (3, 12): LoggingAdapter = logging.LoggerAdapter[logging.Logger] @@ -52,7 +57,7 @@ def getChild( @lru_cache() -def get_logger(name=None) -> logging.Logger: +def get_logger(name: Optional[str] = None) -> logging.Logger: """ Get a `prefect` logger. These loggers are intended for internal use within the `prefect` package. @@ -81,7 +86,9 @@ def get_logger(name=None) -> logging.Logger: return logger -def get_run_logger(context=None, **kwargs): +def get_run_logger( + context: Optional["RunContext"] = None, **kwargs: Any +) -> Union[logging.Logger, LoggingAdapter]: """ Get a Prefect logger for the current task run or flow run. @@ -151,9 +158,9 @@ def get_run_logger(context=None, **kwargs): def flow_run_logger( - flow_run, - flow=None, - **kwargs, + flow_run: Union["FlowRun", "ClientFlowRun"], + flow: Optional["Flow[Any, Any]"] = None, + **kwargs: str, ) -> LoggingAdapter: """ Create a flow run logger with the run's metadata attached. @@ -177,11 +184,11 @@ def flow_run_logger( def task_run_logger( - task_run, - task=None, - flow_run=None, - flow=None, - **kwargs, + task_run: "TaskRun", + task: Optional["Task[Any, Any]"] = None, + flow_run: Optional["FlowRun"] = None, + flow: Optional["Flow[Any, Any]"] = None, + **kwargs: Any, ): """ Create a task run logger with the run's metadata attached. @@ -218,7 +225,7 @@ def task_run_logger( ) -def get_worker_logger(worker, name=None): +def get_worker_logger(worker: "BaseWorker", name: Optional[str] = None): """ Create a worker logger with the worker's metadata attached. @@ -242,7 +249,7 @@ def get_worker_logger(worker, name=None): @contextmanager -def disable_logger(name): +def disable_logger(name: str): """ Get a logger by name and disables it within the context manager. Upon exiting the context manager, the logger is returned to its