Skip to content

Conversation

@Khairajani
Copy link
Contributor

@Khairajani Khairajani commented Jan 22, 2026


Summary by Gitar

  • Fixed datetime compatibility:
    • Added _normalize_datetime_strings() to ensure ISO 8601 timestamps include microseconds (.000000) for Java SimpleDateFormat compatibility
  • New utility function:
    • Recursively processes JSON objects to normalize datetime strings (e.g., 2026-01-19T18:08:41Z2026-01-19T18:08:41.000000Z) before patch generation
  • Updated patch generation:
    • Modified build_patch() in patch_request.py to wrap all json.loads() calls with datetime normalization

This will update automatically on new commits.


@github-actions
Copy link
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@gitar-bot
Copy link

gitar-bot bot commented Jan 22, 2026

🔍 CI failure analysis for b1c775f: All 14 CI job failures are caused by a missing 'safe to test' label on PR #25458, not by code issues. The workflows require this label before executing tests as a security measure.

Issue

All CI jobs failed at the PR label verification step. None of the jobs reached actual test execution.

Root Cause

The workflows use jesusvasquez333/verify-pr-label-action@v1.4.0 to check for a required safe to test label before running tests. PR #25458 does not have this label, causing all jobs to exit with error:

Error! This pull request does not contain any of the valid labels: ['safe to test']
Exiting with an error code

Details

Affected Jobs (14 total):

  • java-checkstyle
  • py-checkstyle
  • py-run-tests (3.10 and 3.11 on both py-tests and py-tests-postgres workflows)
  • py-run-build-tests
  • build-and-scan (both Trivy scan workflows)
  • playwright-ci-postgresql (all 6 shards: 1/6, 2/6, 3/6, 4/6, 5/6, 6/6)

Workflow Pattern:

  1. ✅ Environment setup succeeded
  2. ✅ Wait for "Team Label" check succeeded
  3. ❌ PR label verification failed - missing safe to test label
  4. ⏭️ All test/validation steps skipped

Why This Label Exists:

The safe to test label is a security mechanism to prevent untrusted code from external contributors from automatically running in the CI environment. This protects against malicious code execution, resource abuse, and secrets exposure.

Code Changes:

The PR modifies ingestion/src/metadata/ingestion/models/patch_request.py to normalize datetime strings by ensuring they include microseconds (.000000) for Java SimpleDateFormat compatibility. The changes have not been tested yet because all CI jobs failed before reaching the test phase.

Code Review 👍 Approved with suggestions 0 resolved / 1 findings

Clean fix for datetime normalization with one minor unused import issue.

More details 💡 1 suggestion
💡 Quality: Unused import `Union`

📄 ingestion/src/metadata/ingestion/models/patch_request.py:16

The Union type is added to the imports but is not used anywhere in the visible changes or the new code. This appears to be an accidental addition.

Suggested fix: Remove Union from the import statement:

from typing import Any, Dict, List, Optional, Tuple

What Works Well

The recursive datetime normalization function is well-designed: it properly handles all JSON types, correctly identifies datetime strings missing microseconds via a clear regex pattern, and preserves timezone information. The approach ensures compatibility with Java's SimpleDateFormat requirements.

Rules 🎸 1 action taken

Gitar Rules

🎸 Summary Enhancement: PR description was empty, added comprehensive technical summary

2 rules not applicable. Show all rules by commenting gitar display:verbose.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

)


def _normalize_datetime_strings(obj: Any) -> Any:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if we have any time/datetime utilities module rather than having this on patch_request only. might be useful elsewhere

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants