chore(python): modernize typing and enforce lazy logging by IceS2 · Pull Request #31643 · open-metadata/OpenMetadata

IceS2 · 2026-08-17T13:57:33Z

Describe your changes:

Modernizes Python annotations covered by UP006, UP007, UP035, and UP045, including generated Pydantic classes during make generate. CI now rejects both violations and new # noqa suppressions for those rules.

Enables only G004 in Ruff's selected rules. The current baseline of 2,536 logging f-string violations across 416 files is preserved with targeted # noqa: G004 directives, while new logging f-strings fail lint and must use lazy %-style formatting.

Type of change:

Improvement

High-level design:

Apply Ruff's Python 3.10 typing modernization across handwritten code.
Run the same modernization over generated Python after make generate.
Keep the four typing rules out of Ruff's global selection, but enforce them with --ignore-noqa plus an explicit directive scan so suppressions cannot bypass CI.
Select G004 globally and grandfather the current baseline with targeted directives so future logging code must be lazy-formatted.

This is a mechanical migration; it does not change supported Python versions or public APIs.

Tests:

Use cases covered

Existing Python code contains no UP006, UP007, UP035, or UP045 violations, including suppressed violations.
New suppressions for those four rules are rejected.
New logging f-strings are rejected while the current baseline remains accepted.
Generated Python is modernized on regeneration.

Unit tests

Existing focused ingestion suites: 95 passed.
python -m compileall -q ingestion/src openmetadata-airflow-apis
make py_format_check
Generated Python targeted Ruff check for F401,UP006,UP007,UP035,UP045

The complete unit-test tree could not be collected in the reused local environment because optional connector dependencies are absent and its installed spaCy/Thinc build is binary-incompatible with NumPy.

Backend integration tests

Not applicable (no backend API changes).

Ingestion integration tests

Not applicable (mechanical typing and lint-policy migration).

Playwright (UI) tests

Not applicable (no UI changes).

Manual testing performed

Not applicable.

UI screen recording / screenshots:

Not applicable.

Checklist:

I have read the CONTRIBUTING document.
My PR title is Fixes <issue-number>: <short explanation>.
My PR is linked to a GitHub issue via Fixes #<issue-number> above.
For JSON Schema changes: not applicable, no schema changes.
For UI changes: not applicable, no UI changes.

…n-typing-logging

Copilot

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

github-actions · 2026-08-17T13:57:47Z

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

github-actions · 2026-08-17T13:59:29Z

🔴 Playwright Results — workflow failed

Validated commit 58b8eff97d1b659a656467dd4fc113ef86a9bbe6 in Playwright run 32037324031, attempt 1.

✅ 0 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky

Pipeline and setup failures (6)

The build job finished with status failure.
Duration-aware shard planning finished with status skipped.
Fixture cache restoration finished with status skipped.
Seeded fixture preparation finished with status skipped.
The Playwright shard matrix was unexpectedly skipped.
No expected Playwright shards were declared.

Performance

⚪ Performance metrics unavailable; see the CI and reporting failures above.

Shard	Passed	Failed	Flaky	Skipped	Lifecycle failed	Lifecycle flaky

📦 Download artifacts

How to debug locally

# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

    try to parse it or log the given message
    """
-    logger.debug(f"Parsing message: [{message}]")
+    logger.debug(f"Parsing message: [{message}]")  # noqa: G004


            raise InvalidWorkflowException("Missing type in the serviceConnection config")

-        logger.debug(f"Error parsing the Workflow Configuration for {source_type} ingestion")
+        logger.debug(f"Error parsing the Workflow Configuration for {source_type} ingestion")  # noqa: G004


        ):
            secret_id = self._secret_value.replace(SECRET, "")
-            logger.info(f"Getting secret value for {secret_id}")
+            logger.info(f"Getting secret value for {secret_id}")  # noqa: G004


                secret_value = SecretsManagerFactory().get_secrets_manager().get_string_value(secret_id)
            except Exception as exc:
-                logger.error(f"Secret value [{secret_id}] not present in the configured secrets manager: {exc}")
+                logger.error(f"Secret value [{secret_id}] not present in the configured secrets manager: {exc}")  # noqa: G004


@@ -107,11 +107,11 @@
        """
        try:
            secret: KeyVaultSecret = self.client.get_secret(secret_id)
-            logger.debug(f"Got value for secret {secret_id}")
+            logger.debug(f"Got value for secret {secret_id}")  # noqa: G004


@@ -134,24 +133,24 @@
            # Kubernetes stores secret data as base64 encoded
            if secret.data and "value" in secret.data:
                secret_value = base64.b64decode(secret.data["value"]).decode("utf-8")
-                logger.debug(f"Got value for secret {secret_id}")
+                logger.debug(f"Got value for secret {secret_id}")  # noqa: G004


                return secret_value
-            logger.warning(f"Secret {secret_id} exists but has no 'value' key")
+            logger.warning(f"Secret {secret_id} exists but has no 'value' key")  # noqa: G004


            return None  # noqa: TRY300

        except ApiException as exc:
            if exc.status == 404:
-                logger.debug(f"Secret {secret_id} not found")
+                logger.debug(f"Secret {secret_id} not found")  # noqa: G004


                return None
            logger.debug(traceback.format_exc())
-            logger.error(f"Could not get the secret value of {secret_id} due to [{exc}]")
+            logger.error(f"Could not get the secret value of {secret_id} due to [{exc}]")  # noqa: G004


            raise exc  # noqa: TRY201
        except Exception as exc:
            logger.debug(traceback.format_exc())
-            logger.error(f"Could not get the secret value of {secret_id} due to [{exc}]")
+            logger.error(f"Could not get the secret value of {secret_id} due to [{exc}]")  # noqa: G004


greptile-apps · 2026-08-17T14:17:04Z

Too many files changed for review (1022 files, 100 file limit).

gitar-bot · 2026-08-17T14:21:46Z

+	@if rg -n --glob '*.py' '[#][[:space:]]*noqa:[^#]*(UP006|UP007|UP035|UP045)' $(INGESTION_DIR)/ $(ROOT_DIR)/openmetadata-airflow-apis/; then \
+		echo "Do not suppress modern typing rules; fix the annotation instead." >&2; \
+		exit 1; \
+	fi
+	ruff check --ignore-noqa --select $(MODERN_TYPING_RULES) $(INGESTION_DIR)/ $(ROOT_DIR)/openmetadata-airflow-apis/ --config $(INGESTION_DIR)/pyproject.toml


💡 Quality: noqa-scan guard silently skips if ripgrep is unavailable

The new guard in ingestion/Makefile (if rg -n ... '(UP006|UP007|UP035|UP045)'; then ...exit 1; fi) relies on rg's exit code: 0 = matches found (fail), 1 = no matches (pass). But if rg is missing or errors, it exits with code 2, which the if treats as falsey and the build proceeds — silently disabling the anti-suppression enforcement rather than failing loudly. rg is used elsewhere so this is low-risk, but pinning rg as an explicit prerequisite or checking command -v rg before the scan would make the guard fail-closed. Minor; the primary ruff check --ignore-noqa --select step below still enforces the rules regardless.

_{Was this helpful? React with 👍 / 👎}

Copilot

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

gitar-bot · 2026-08-17T16:59:35Z

Code Review 👍 Approved with suggestions 0 resolved / 1 findings

Modernizes Python typing annotations and enforces lazy logging formatting across ingestion codebases. Consider adding a fallback for the noqa-scan guard when ripgrep is unavailable.

💡 Quality: noqa-scan guard silently skips if ripgrep is unavailable

📄 ingestion/Makefile:63-67

The new guard in ingestion/Makefile (if rg -n ... '(UP006|UP007|UP035|UP045)'; then ...exit 1; fi) relies on rg's exit code: 0 = matches found (fail), 1 = no matches (pass). But if rg is missing or errors, it exits with code 2, which the if treats as falsey and the build proceeds — silently disabling the anti-suppression enforcement rather than failing loudly. rg is used elsewhere so this is low-risk, but pinning rg as an explicit prerequisite or checking command -v rg before the scan would make the guard fail-closed. Minor; the primary ruff check --ignore-noqa --select step below still enforces the rules regardless.

🤖 Prompt for agents

Code Review: Modernizes Python typing annotations and enforces lazy logging formatting across ingestion codebases. Consider adding a fallback for the noqa-scan guard when ripgrep is unavailable.

1. 💡 Quality: noqa-scan guard silently skips if ripgrep is unavailable
   Files: ingestion/Makefile:63-67

   The new guard in ingestion/Makefile (`if rg -n ... '(UP006|UP007|UP035|UP045)'; then ...exit 1; fi`) relies on rg's exit code: 0 = matches found (fail), 1 = no matches (pass). But if rg is missing or errors, it exits with code 2, which the `if` treats as falsey and the build proceeds — silently disabling the anti-suppression enforcement rather than failing loudly. rg is used elsewhere so this is low-risk, but pinning rg as an explicit prerequisite or checking `command -v rg` before the scan would make the guard fail-closed. Minor; the primary `ruff check --ignore-noqa --select` step below still enforces the rules regardless.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

`Compact`
`gitar display:verbose`

_{Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source}

IceS2 added 3 commits August 17, 2026 15:52

chore(python): modernize typing and enforce lazy logging

e980667

Merge remote-tracking branch 'origin/main' into chore/modernize-pytho…

42298aa

…n-typing-logging

chore(python): align latest main with lint policy

58b8eff

IceS2 requested a review from a team as a code owner August 17, 2026 13:57

Copilot AI lite review requested due to automatic review settings August 17, 2026 13:57

Copilot AI reviewed Aug 17, 2026

github-actions Bot added Ingestion safe to test Add this label to run secure Github workflows on PRs labels Aug 17, 2026

IceS2 added this to Shipping Aug 17, 2026

github-advanced-security Bot found potential problems Aug 17, 2026

View reviewed changes

gitar-bot Bot reviewed Aug 17, 2026

View reviewed changes

chore(python): restore tag processor print exemption

75d87f4

Copilot AI review requested due to automatic review settings August 17, 2026 16:58

Copilot AI reviewed Aug 17, 2026

IceS2 temporarily deployed to test August 17, 2026 17:31 — with GitHub Actions Inactive

Navigation Menu

Uh oh!

chore(python): modernize typing and enforce lazy logging - #31643

chore(python): modernize typing and enforce lazy logging#31643
IceS2 wants to merge 4 commits into
mainfrom
chore/modernize-python-typing-logging

IceS2 commented Aug 17, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented Aug 17, 2026

Uh oh!

github-actions Bot commented Aug 17, 2026

Uh oh!

greptile-apps Bot commented Aug 17, 2026 •

edited

Loading

Uh oh!

gitar-bot Bot Aug 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

gitar-bot Bot commented Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

IceS2 commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your changes:

Type of change:

High-level design:

Tests:

Use cases covered

Unit tests

Backend integration tests

Ingestion integration tests

Playwright (UI) tests

Manual testing performed

UI screen recording / screenshots:

Checklist:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 17, 2026

❌ PR checklist incomplete

Uh oh!

github-actions Bot commented Aug 17, 2026

🔴 Playwright Results — workflow failed

Pipeline and setup failures (6)

Performance

Uh oh!

greptile-apps Bot commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gitar-bot Bot Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

gitar-bot Bot commented Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

IceS2 commented Aug 17, 2026 •

edited

Loading

greptile-apps Bot commented Aug 17, 2026 •

edited

Loading