From c09e4bab978b2a1970a3dd194c3041dd434675f1 Mon Sep 17 00:00:00 2001 From: Alex Streed Date: Tue, 10 Dec 2024 14:09:08 -0600 Subject: [PATCH 1/9] Include more helpful output in type completeness check --- .github/workflows/static-analysis.yaml | 1 + scripts/pyright_diff.py | 88 ++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 scripts/pyright_diff.py diff --git a/.github/workflows/static-analysis.yaml b/.github/workflows/static-analysis.yaml index 2df6ed87ae3c..c9f9ff18409d 100644 --- a/.github/workflows/static-analysis.yaml +++ b/.github/workflows/static-analysis.yaml @@ -110,6 +110,7 @@ jobs: 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 + uv run scripts/pyright_diff.py prefect-analysis-base.json prefect-analysis.json >> $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 diff --git a/scripts/pyright_diff.py b/scripts/pyright_diff.py new file mode 100644 index 000000000000..86b1f12696d5 --- /dev/null +++ b/scripts/pyright_diff.py @@ -0,0 +1,88 @@ +import json +import sys +from typing import Any, Dict, NamedTuple + + +class Diagnostic(NamedTuple): + """Structured representation of a diagnostic for easier table formatting.""" + + file: str + line: int + character: int + severity: str + message: str + + +def normalize_diagnostic(diagnostic: Dict[Any, Any]) -> Dict[Any, Any]: + """Normalize a diagnostic by removing or standardizing volatile fields.""" + normalized = diagnostic.copy() + normalized.pop("time", None) + normalized.pop("version", None) + return normalized + + +def load_and_normalize_file(file_path: str) -> Dict[Any, Any]: + """Load a JSON file and normalize its contents.""" + with open(file_path, "r") as f: + data = json.load(f) + return normalize_diagnostic(data) + + +def parse_diagnostic(diag: Dict[Any, Any]) -> Diagnostic: + """Convert a diagnostic dict into a Diagnostic object.""" + file = diag.get("file", "unknown_file") + message = diag.get("message", "no message") + range_info = diag.get("range", {}) + start = range_info.get("start", {}) + line = start.get("line", 0) + char = start.get("character", 0) + severity = diag.get("severity", "unknown") + + return Diagnostic(file, line, char, severity, message) + + +def format_markdown_table(diagnostics: list[Diagnostic]) -> str: + """Format list of diagnostics as a markdown table.""" + if not diagnostics: + return "\nNo new errors found!" + + table = ["| File | Location | Message |", "|------|----------|---------|"] + + for diag in sorted(diagnostics, key=lambda x: (x.file, x.line, x.character)): + # Escape pipe characters in the message to prevent table formatting issues + message = diag.message.replace("|", "\\|") + location = f"L{diag.line}:{diag.character}" + table.append(f"| {diag.file} | {location} | {message} |") + + return "\n".join(table) + + +def compare_pyright_outputs(base_file: str, new_file: str) -> None: + """Compare two pyright JSON output files and display only new errors.""" + base_data = load_and_normalize_file(base_file) + new_data = load_and_normalize_file(new_file) + + # Group diagnostics by file + base_diags = set() + new_diags = set() + + # Process diagnostics from type completeness symbols + for data, diag_set in [(base_data, base_diags), (new_data, new_diags)]: + for symbol in data.get("typeCompleteness", {}).get("symbols", []): + for diag in symbol.get("diagnostics", []): + if diag.get("severity", "") == "error": + diag_set.add(parse_diagnostic(diag)) + + # Find new errors + new_errors = list(new_diags - base_diags) + + print("\n## New Pyright Errors\n") + print(format_markdown_table(new_errors)) + + +if __name__ == "__main__": + if len(sys.argv) != 3: + print("Usage: python pyright_diff.py ") + sys.exit(1) + + compare_pyright_outputs(sys.argv[1], sys.argv[2]) From f2c42e6abd079cc7c0e48ebd3b4557d7e9bd04e0 Mon Sep 17 00:00:00 2001 From: Alex Streed Date: Tue, 10 Dec 2024 14:10:47 -0600 Subject: [PATCH 2/9] Introduce a typing error --- src/prefect/logging/loggers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/prefect/logging/loggers.py b/src/prefect/logging/loggers.py index 4a9211a8d95b..c7f9e33edbf4 100644 --- a/src/prefect/logging/loggers.py +++ b/src/prefect/logging/loggers.py @@ -188,7 +188,7 @@ def task_run_logger( task: Optional["Task[Any, Any]"] = None, flow_run: Optional["FlowRun"] = None, flow: Optional["Flow[Any, Any]"] = None, - **kwargs: Any, + **kwargs, ): """ Create a task run logger with the run's metadata attached. From 3660e58b7fac4965e4c67be00907f9b9f232fbe5 Mon Sep 17 00:00:00 2001 From: Alex Streed Date: Tue, 10 Dec 2024 14:15:34 -0600 Subject: [PATCH 3/9] Remove more type hints for testing --- src/prefect/logging/loggers.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/prefect/logging/loggers.py b/src/prefect/logging/loggers.py index c7f9e33edbf4..bc577d888a93 100644 --- a/src/prefect/logging/loggers.py +++ b/src/prefect/logging/loggers.py @@ -19,7 +19,6 @@ 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] @@ -225,7 +224,7 @@ def task_run_logger( ) -def get_worker_logger(worker: "BaseWorker", name: Optional[str] = None): +def get_worker_logger(worker, name: Optional[str] = None): """ Create a worker logger with the worker's metadata attached. @@ -249,7 +248,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 @@ -279,7 +278,7 @@ def disable_run_logger(): yield -def print_as_log(*args: Any, **kwargs: Any) -> None: +def print_as_log(*args, **kwargs) -> None: """ A patch for `print` to send printed messages to the Prefect run logger. From 16b43fae1e727e7757a6e110a2e24c57610df857 Mon Sep 17 00:00:00 2001 From: Alex Streed Date: Tue, 10 Dec 2024 14:21:21 -0600 Subject: [PATCH 4/9] Try a different path --- .github/workflows/static-analysis.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/static-analysis.yaml b/.github/workflows/static-analysis.yaml index c9f9ff18409d..e13053ff2d62 100644 --- a/.github/workflows/static-analysis.yaml +++ b/.github/workflows/static-analysis.yaml @@ -110,7 +110,7 @@ jobs: 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 - uv run scripts/pyright_diff.py prefect-analysis-base.json prefect-analysis.json >> $GITHUB_STEP_SUMMARY + uv run ./scripts/pyright_diff.py prefect-analysis-base.json prefect-analysis.json >> $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 From a859c45bd707c225e48ca61f3298e53377e15cb9 Mon Sep 17 00:00:00 2001 From: Alex Streed Date: Tue, 10 Dec 2024 14:32:40 -0600 Subject: [PATCH 5/9] Fix thing --- .github/workflows/static-analysis.yaml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/static-analysis.yaml b/.github/workflows/static-analysis.yaml index e13053ff2d62..7cd15e18db3c 100644 --- a/.github/workflows/static-analysis.yaml +++ b/.github/workflows/static-analysis.yaml @@ -102,6 +102,10 @@ jobs: BASE_SCORE=$(jq -r '.typeCompleteness.completenessScore' prefect-analysis-base.json) echo "base_score=$BASE_SCORE" >> $GITHUB_OUTPUT + - name: Checkout current branch + run: | + git checkout ${{ github.ref }} + - name: Compare scores run: | CURRENT_SCORE=$(echo ${{ steps.calculate_current_score.outputs.current_score }}) @@ -110,7 +114,7 @@ jobs: 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 - uv run ./scripts/pyright_diff.py prefect-analysis-base.json prefect-analysis.json >> $GITHUB_STEP_SUMMARY + uv run scripts/pyright_diff.py prefect-analysis-base.json prefect-analysis.json >> $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 From 9e2e1d28ea50156f109457ca9a91451a39389533 Mon Sep 17 00:00:00 2001 From: Alex Streed Date: Tue, 10 Dec 2024 14:35:33 -0600 Subject: [PATCH 6/9] Checkout differently --- .github/workflows/static-analysis.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/static-analysis.yaml b/.github/workflows/static-analysis.yaml index 7cd15e18db3c..45eb836fadc7 100644 --- a/.github/workflows/static-analysis.yaml +++ b/.github/workflows/static-analysis.yaml @@ -104,7 +104,7 @@ jobs: - name: Checkout current branch run: | - git checkout ${{ github.ref }} + git checkout ${{ github.head_ref || github.ref_name }} - name: Compare scores run: | From 857cf3adff60c76c8af2a27305ffaf70507cb45e Mon Sep 17 00:00:00 2001 From: Alex Streed Date: Tue, 10 Dec 2024 14:44:25 -0600 Subject: [PATCH 7/9] Escape newlines --- scripts/pyright_diff.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/pyright_diff.py b/scripts/pyright_diff.py index 86b1f12696d5..b4b87ada26d6 100644 --- a/scripts/pyright_diff.py +++ b/scripts/pyright_diff.py @@ -52,7 +52,7 @@ def format_markdown_table(diagnostics: list[Diagnostic]) -> str: # Escape pipe characters in the message to prevent table formatting issues message = diag.message.replace("|", "\\|") location = f"L{diag.line}:{diag.character}" - table.append(f"| {diag.file} | {location} | {message} |") + table.append(f"| {diag.file} | {location} | {message.replace('\n', '\\n')} |") return "\n".join(table) From 6bde40055234a6f05bbbbfe2d530478f580eeed4 Mon Sep 17 00:00:00 2001 From: Alex Streed Date: Tue, 10 Dec 2024 14:47:34 -0600 Subject: [PATCH 8/9] Do it a different way --- scripts/pyright_diff.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/pyright_diff.py b/scripts/pyright_diff.py index b4b87ada26d6..ceaf43a1601b 100644 --- a/scripts/pyright_diff.py +++ b/scripts/pyright_diff.py @@ -49,10 +49,10 @@ def format_markdown_table(diagnostics: list[Diagnostic]) -> str: table = ["| File | Location | Message |", "|------|----------|---------|"] for diag in sorted(diagnostics, key=lambda x: (x.file, x.line, x.character)): - # Escape pipe characters in the message to prevent table formatting issues - message = diag.message.replace("|", "\\|") + # Escape pipe characters and replace newlines with HTML breaks + message = diag.message.replace("|", "\\|").replace("\n", "
") location = f"L{diag.line}:{diag.character}" - table.append(f"| {diag.file} | {location} | {message.replace('\n', '\\n')} |") + table.append(f"| {diag.file} | {location} | {message} |") return "\n".join(table) From 0d81befd32f7b50db49a6afd51f7b49223d16def Mon Sep 17 00:00:00 2001 From: Alex Streed Date: Tue, 10 Dec 2024 14:49:41 -0600 Subject: [PATCH 9/9] Revert type errors --- src/prefect/logging/loggers.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/prefect/logging/loggers.py b/src/prefect/logging/loggers.py index bc577d888a93..4a9211a8d95b 100644 --- a/src/prefect/logging/loggers.py +++ b/src/prefect/logging/loggers.py @@ -19,6 +19,7 @@ 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] @@ -187,7 +188,7 @@ def task_run_logger( task: Optional["Task[Any, Any]"] = None, flow_run: Optional["FlowRun"] = None, flow: Optional["Flow[Any, Any]"] = None, - **kwargs, + **kwargs: Any, ): """ Create a task run logger with the run's metadata attached. @@ -224,7 +225,7 @@ def task_run_logger( ) -def get_worker_logger(worker, name: Optional[str] = None): +def get_worker_logger(worker: "BaseWorker", name: Optional[str] = None): """ Create a worker logger with the worker's metadata attached. @@ -248,7 +249,7 @@ def get_worker_logger(worker, name: Optional[str] = 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 @@ -278,7 +279,7 @@ def disable_run_logger(): yield -def print_as_log(*args, **kwargs) -> None: +def print_as_log(*args: Any, **kwargs: Any) -> None: """ A patch for `print` to send printed messages to the Prefect run logger.