Skip to content

chore: remove unused lastSuccessfulTime field from CronOMJobStatus - #32780

Open
detail-app[bot] wants to merge 1 commit into
mainfrom
detail/dead-code/chore-remove-unused-lastsuccessfultime-field-from-622c0c
Open

chore: remove unused lastSuccessfulTime field from CronOMJobStatus#32780
detail-app[bot] wants to merge 1 commit into
mainfrom
detail/dead-code/chore-remove-unused-lastsuccessfultime-field-from-622c0c

Conversation

@detail-app

@detail-app detail-app Bot commented Sep 7, 2026

Copy link
Copy Markdown

Describe your changes:

Removed the lastSuccessfulTime field — its @JsonProperty/@JsonFormat annotations, the private Instant lastSuccessfulTime declaration, and the getLastSuccessfulTime() / setLastSuccessfulTime() accessors — from CronOMJobStatus in the openmetadata-k8s-operator module.

I worked on removing dead code from the k8s-operator status model because the lastSuccessfulTime field has zero callers and is never persisted to Kubernetes.

Type of change:

  • Improvement

High-level design:

N/A — small change.

Why this is dead code

CronOMJobStatus is the status sub-resource type of the CronOMJob Kubernetes Custom Resource (CronOMJobResource extends CustomResource<CronOMJobSpec, CronOMJobStatus>). It exposes four fields via @JsonProperty; of these, only lastSuccessfulTime is dead:

  • No production call-sites: neither getLastSuccessfulTime() nor setLastSuccessfulTime() is invoked anywhere in the codebase. The reconciler (CronOMJobReconciler) reads/writes lastScheduleTime, lastOMJobName, and message but never touches lastSuccessfulTime.
  • No test references: the field is not exercised by unit tests (CronOMJobReconcilerTest, CRDSchemaValidationTest) or integration tests (K8sOMJobOperatorIT).
  • Absent from the CRD schema: openmetadata-k8s-operator/src/main/resources/crds/cronomjob-crd.yaml declares lastScheduleTime, lastOMJobName, and message under status but omits lastSuccessfulTime, so Kubernetes would prune it from the status sub-resource even if it were set.
  • Not referenced in the README (openmetadata-service/.../k8s/README.md mentions only lastScheduleTime and lastOMJobName).

Since the field is never written, it always serializes to null and carries no behavior. Removing it makes the status model accurately reflect what the operator actually tracks, avoiding the misleading impression that a "last successful time" feature is implemented.

Tests:

Use cases covered

No behavioral change — the removed field was never read or written, so no user-visible scenario is affected.

Unit tests

  • Not applicable (no behavioral change). Existing tests suffice.
  • Verified: mvn test -pl openmetadata-k8s-operator → 66 tests, 0 failures, 0 errors (includes CronOMJobReconcilerTest and CRDSchemaValidationTest, both of which construct CronOMJobStatus).

Backend integration tests

  • Not applicable (no backend API changes).

Ingestion integration tests

  • Not applicable (no ingestion changes).

Playwright (UI) tests

  • Not applicable (no UI changes).

Manual testing performed

  1. mvn compile -pl openmetadata-k8s-operator -q → success.
  2. mvn test -pl openmetadata-k8s-operator -DfailIfNoTests=false → 66 tests, 0 failures, 0 errors.

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.
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.
  • For UI changes: I attached a screen recording and/or screenshots above.
  • I have added tests (unit / integration / Playwright as applicable) and listed them above.

History


Dead Code PRs can be configured here.

@detail-app
detail-app Bot requested a review from pmbrull September 7, 2026 00:24
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
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!

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit 608d3b792341725c185d7c4dab1dfb2eda24f89a in Playwright run 34071309043, attempt 1.

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

Performance

Blocking targets: ✅ met · Optimization targets: 🟡 in progress

Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting.

🕒 Full workflow signal wall (to summary) 45m 41s

⏱️ Max setup 4m 13s · max shard execution 17m 37s · max shard-job elapsed before upload 21m 20s · reporting 4s

🌐 234.87 requests/attempt · 2.82 app boots/UI scenario · 8.61% common-shard skew

Optimization targets still in progress:

  • Browser traffic was 234.87 requests per attempt (convergence target: fewer than 200).
  • Application boot ratio was 2.82 per UI scenario (1629 boots / 578 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard chromium-01 147 0 0 0 0 0
✅ Shard chromium-02 130 0 0 0 0 0
✅ Shard chromium-03 122 0 0 0 0 0
✅ Shard data-asset-rules-01 65 0 0 0 0 0
✅ Shard domain-isolation-01 16 0 0 0 0 0
✅ Shard global-state-01 34 0 0 0 0 0
✅ Shard ingestion-01 1 0 0 0 0 0
✅ Shard reindex-01 2 0 0 0 0 0
✅ Shard search-01 11 0 0 0 0 0
✅ Shard search-rbac-01 29 0 0 0 0 0

📦 Download artifacts

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

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

Labels

safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant