Skip to content

feat(ingestion-pipelines): add agentType filter and stop polling orchestrators for queued status - #31606

Open
pmbrull wants to merge 3 commits into
mainfrom
pmbrull/agent-type-filter-pipeline-runs
Open

feat(ingestion-pipelines): add agentType filter and stop polling orchestrators for queued status#31606
pmbrull wants to merge 3 commits into
mainfrom
pmbrull/agent-type-filter-pipeline-runs

Conversation

@pmbrull

@pmbrull pmbrull commented Aug 17, 2026 •
edited by greptile-apps Bot
Loading

Copy link
Copy Markdown
Member

Describe your changes

Two changes to the ingestion pipeline APIs behind the service Agents page, plus a crash found while testing them.

1. agentType list filter

GET /v1/services/ingestionPipelines had no way to ask for "the metadata agents", so every caller enumerated the seven pipelineType values that make one up. The UI kept that list in SERVICE_INGESTION_PIPELINE_TYPES, Collate kept its own copy, and a caller that passed nothing got AI automations (pipelineType=application) mixed into the Metadata tab:

/api/v1/services/ingestionPipelines?fields=owners,pipelineStatuses&service=banking-bigquery&serviceType=databaseService

Now:

filter expands to
agentType=metadata metadata, usage, lineage, profiler, autoClassification, dbt, policyAgent
agentType=application application

It is an allow-list, not a negation. TestSuite, dataInsight and elasticSearchReindex belong to neither group; implementing metadata as "everything that is not an application" would leak them onto the page. Setting both agentType and pipelineType intersects them, so a caller can narrow a group to one type; an empty intersection matches nothing.

The filter is derived, not stored — AgentTypeResolver expands it into the existing pipelineType IN (...) clause. No new entity field, no migration, and nothing that can drift out of sync with pipelineType.

2. Queued status without an orchestrator round trip

IngestionPipelineRepository.listPipelineStatus called pipelineServiceClient.getQueuedPipelineStatus on every read. For the K8s client that is a listNamespacedJob against the API server each time run history is opened, including paged historical reads that cannot contain a queued run. The Airflow call has a 10s connect timeout and no read timeout, so a slow scheduler pins the Jetty thread.

PipelineServiceClientResponse now carries runId:

PipelineServiceClientResponse response = pipelineServiceClient.runPipeline(ingestionPipeline, service);
repository.recordQueuedPipelineStatus(uriInfo, ingestionPipeline.getFullyQualifiedName(), response.getRunId());

Orchestrators that mint the run ID when triggering report it back, and the server records the queued status itself. The K8s client already generated that ID and injects it into the pod as pipelineRunId, so the worker's later status upserts onto the same row (addPipelineStatus keys on runId) — no phantom run. Airflow mints its run ID inside the task (openmetadata_managed_apis/workflows/ingestion/common.py) and cannot report one, so it returns null and keeps the live-polling path completely unchanged.

No engine branching anywhere — the presence of a runId selects the behaviour. Collate's ArgoServiceClient gets the same treatment in a companion PR.

Recorded on three trigger paths, not just the manual one: IngestionPipelineResource (Run button), AppResource (Applications page) and RunIngestionPipelineImpl (AutoPilot, whose runs render on the very Agents page this change is about). Without them those runs would lose the indicator along with the poll, since listPipelineStatus is the single read path for queued status regardless of who triggered the run.

Deliberately not recorded on RunAppImpl or DataContractRepository. Those are workflow-internal: waitForCompletion treats any non-terminal status as still running, so the queued row buys their own polling nothing while costing an upsert plus a search reindex per run.

3. Crash fix

getOperationForPipelineType derived the workflow type outside its own try block, so an application pipeline with no appConfig threw NullPointerException before authorization ran and returned 500, instead of falling back to the generic create permission the way every other unrecognized type already does.

Type of change:

  • Improvement

High-level design:

Why agentType is a query-param preset rather than an entity field. It carries zero information that is not already derivable from pipelineType. Storing it would mean a schema field, a migration, a backfill, and two sources of truth that can disagree. Expanding it in ListFilter's existing pipelineType IN (...) path costs nothing and cannot drift.

Why the run ID lives on the response instead of a capability flag. The alternatives were an instanceof check in the repository, or a supportsStoredQueuedStatus() method on the client interface. Both make the repository know about engines. A nullable runId on the response is self-selecting: an orchestrator that knows its run ID says so, one that does not stays on the old path, and a future engine opts in by populating one field.

Rejected: correlating by writing the queued row under a server-generated ID. For Airflow the server never learns the ID the worker will use, so the queued row would never resolve and run history would show a permanently-pending phantom next to the real run. Passing a run_id through trigger.py into pipelineRunId would fix that, but it spans three repos and needs a version gate for older openmetadata-airflow-apis that ignore the parameter. Not worth it to save one HTTP call on the engine that is least affected.

Trade-off — a run that is accepted but never starts. The live poll was self-cleaning: it re-derived queued state from the orchestrator each read. A stored row has no such feedback, so an unschedulable job (no capacity, ImagePullBackOff, quota) would stay queued forever. Queued rows older than queuedStatusTimeoutSeconds (new config, default 1h) are therefore hidden on read. The default is deliberately generous so a genuinely long queue does not disappear from the UI.

Known gap — scheduled runs. They generate their run ID outside the trigger path: CronOMJob via a reconciler placeholder, plain CronJob via the pod UID (Downward API). They get no queued row and appear when the worker posts its first status. Also lost: queued runs OpenMetadata did not trigger, e.g. a DAG started directly in the Airflow UI. Both were judged acceptable against the per-read cost.

Backward compatibility. agentType is optional and additive; omitting it preserves today's behaviour exactly. runId is an optional response field. queuedStatusTimeoutSeconds defaults to 3600. Hybrid deployments are untouched — HybridServiceClient overrides getQueuedPipelineStatus and its runner response carries no runId, so those pipelines keep their existing path.

Tests:

Use cases covered

  • The service Agents page lists only metadata agents, with AI automations excluded, without the client enumerating pipeline types
  • A pipelineType that belongs to no agent group (elasticSearchReindex) is excluded from both groups rather than falling into metadata
  • agentType combined with pipelineType narrows to the intersection; a disjoint combination returns nothing
  • Opening run history no longer issues a Kubernetes API call
  • A manually triggered K8s run reports its run ID so the server can record queued, and the worker's status collapses onto that same row
  • A run accepted but never started stops showing as queued after the timeout
  • Creating an application pipeline with no appConfig succeeds instead of returning 500

Unit tests

  • Added — AgentTypeResolverTest (7), IngestionPipelineStaleQueuedStatusTest (4), plus updated K8sPipelineClientTest
  • AgentTypeResolverTest includes a guard asserting the exact set of pipeline types excluded from metadata, so a newly added PipelineType fails the build until someone classifies it rather than silently vanishing from the page.
  • Affected suites run green: AgentTypeResolverTest, IngestionPipelineStaleQueuedStatusTest, K8sPipelineClientTest, AirflowRESTClientTest, LogStorageTest, PipelineServiceClientTest, RunIngestionPipelineImplTest, IngestionPipelineRepositoryTest, DataContractFieldSupportTest, AppResourceRetryQueueTest, AppResourceSnapshotRedactionTest — 156 tests, 0 failures.

Integration tests

  • Added — IngestionPipelineAgentTypeIT (2 tests, real server + MySQL + Elasticsearch via Testcontainers): green.
  • Added — IngestionPipelineResourceIT#test_createApplicationPipelineWithoutAppConfig, a regression test for the NPE. Verified RED by stashing only IngestionPipelineResource.java and rebuilding: fails with the exact NullPointerException at getOperationForPipelineType. Green with the fix.
  • Full IngestionPipelineResourceIT: 233 tests, 0 failures (20 pre-existing skips).

Frontend tests

  • ServiceDetailsPage Jest suite: 54 tests, 0 failures.

Manual test steps

The agentType filter is exercised end-to-end by IngestionPipelineAgentTypeIT against a real server rather than by hand. The trigger → queued → worker-status transition needs a live K8s or Argo orchestrator, which neither the IT stack (Airflow client pointed at nothing) nor Jest can provide; it is covered structurally instead — K8sPipelineClientTest asserts the run ID is reported on the response and that the queued read costs no API call, and addPipelineStatus's existing upsert-by-runId is what collapses the rows. Worth a look on a real cluster before release.

UI screen recording / screenshots

No visual change. The only UI diff swaps the query the Agents tab sends:

-  pipelineType: SERVICE_INGESTION_PIPELINE_TYPES,
+  agentType: AgentType.Metadata,

The rendered list is identical — same pipelines, same order. SERVICE_INGESTION_PIPELINE_TYPES stays for its two non-filter uses (building the "Add Agent" menu, and filtering pipelines discovered over SSE in useMetadataAgents), which cannot move server-side.

Checklist:

  • Generated TypeScript types regenerated from the schema changes (json2ts-generate-all.sh) — 4 files, no unrelated drift
  • mvn spotless:check clean
  • UI ESLint + Prettier + organize-imports clean (0 errors)
  • tsc --noEmit reports nothing in the changed files
  • No .github/workflows/** changes
  • Linked issue — none; happy to open one if you want the discussion recorded

🤖 Generated with Claude Code

Greptile Summary

The PR adds server-side agent-type filtering and replaces Kubernetes queued-status polling with persisted queued statuses keyed by orchestrator run ID. It also adds expiration for stale queued statuses, extends trigger paths to record queued runs, and handles application pipelines without app configuration.

  • Adds the metadata and application agent groups and updates the Agents page query.
  • Propagates Kubernetes run IDs and records queued status after manual, application, and workflow-triggered runs.
  • Adds configurable stale-queued visibility and coverage for filtering, status handling, and authorization fallback.

Confidence Score: 4/5

The PR is not yet safe to merge because stale queued rows can still displace valid runs from limited history responses.

The database limit is applied before stale queued statuses are filtered, and the repository does not fetch replacement rows afterward, so valid older history entries can be omitted.

Files Needing Attention: openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/IngestionPipelineRepository.java

Important Files Changed
Filename Overview
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/IngestionPipelineRepository.java Adds persisted queued-status recording and timeout-based filtering across status response paths.
openmetadata-service/src/main/java/org/openmetadata/service/clients/pipeline/k8s/K8sPipelineClient.java Returns the generated run ID from triggers and removes Kubernetes API polling from queued-status reads.
openmetadata-service/src/main/java/org/openmetadata/service/resources/services/ingestionpipelines/AgentTypeResolver.java Defines explicit agent-group allow-lists and intersection behavior for pipeline-type filtering.
openmetadata-service/src/main/java/org/openmetadata/service/resources/services/ingestionpipelines/IngestionPipelineResource.java Exposes the agentType filter, records queued statuses after triggers, and safely falls back to generic create authorization.
openmetadata-ui/src/main/resources/ui/src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx Replaces the client-maintained metadata pipeline-type list with the new server-side agentType filter.

Sequence Diagram

 
sequenceDiagram
  participant UI as Agents UI
  participant API as Ingestion Pipeline API
  participant Repo as Pipeline Repository
  participant K8s as Kubernetes Client
  participant DB as Status Store
  UI->>API: List pipelines with agentType
  API->>Repo: Resolve agentType to pipelineType set
  Repo-->>UI: Filtered pipelines
  UI->>API: Trigger pipeline
  API->>K8s: runPipeline
  K8s-->>API: response with runId
  API->>Repo: recordQueuedPipelineStatus(runId)
  Repo->>DB: Upsert queued status
  Note over DB: Worker updates the same runId later
Loading

Reviews (3): Last reviewed commit: "fix(ingestion-pipelines): apply the stal..." | Re-trigger Greptile

Context used (4)

@pmbrull @claude
feat(ingestion-pipelines): add agentType filter and stop polling orch…
519385f 
…estrators for queued status

Two changes to the ingestion pipeline APIs the Agents page depends on, plus
a crash found while testing them.

**agentType list filter.** `GET /v1/services/ingestionPipelines` had no way to
ask for "the metadata agents", so every caller enumerated the seven
pipelineTypes that make one up. The UI kept that list in
`SERVICE_INGESTION_PIPELINE_TYPES` and Collate kept its own copy, and a
caller that passed nothing got AI automations mixed into the Metadata tab.
`agentType=metadata` now expands server-side to the same explicit allow-list;
`agentType=application` maps to `application`. It is an allow-list, not
"everything that is not an application" — TestSuite, dataInsight and
elasticSearchReindex belong to neither group and must not leak onto the page.
Setting both `agentType` and `pipelineType` intersects them.

The filter is derived, not stored: it expands into the existing
`pipelineType IN (...)` clause, so there is no new entity field, no migration
and nothing that can drift from pipelineType.

**Queued status without an orchestrator round trip.** `listPipelineStatus`
called `pipelineServiceClient.getQueuedPipelineStatus` on every read, which
for the K8s client meant a `listNamespacedJob` against the API server each
time run history was opened — including paged historical reads that cannot
contain a queued run.

`PipelineServiceClientResponse` now carries `runId`. Orchestrators that mint
the run ID when triggering report it back and the server records the queued
status itself; the K8s client already generated that ID and injects it into
the pod as `pipelineRunId`, so the worker's later status upserts onto the same
row. Airflow mints its run ID inside the task and cannot report one, so it
returns null and keeps the live-polling path unchanged. No engine branching is
needed anywhere — the presence of a runId selects the behaviour.

Every server-side trigger records the queued status, not just the manual one,
so app, governance-workflow and data-contract runs do not lose the indicator
along with the poll. A run the orchestrator accepts but never starts would
otherwise stay queued forever now that nothing re-checks it, so queued rows
older than `queuedStatusTimeoutSeconds` (default 1h) are hidden on read.

Scheduled runs generate their run ID outside the trigger path — CronOMJob via
a reconciler placeholder, plain CronJob via the pod UID — so they get no
queued row and appear when the worker posts its first status.

**Crash fix.** `getOperationForPipelineType` derived the workflow type outside
its own try block, so an application pipeline with no `appConfig` threw NPE
before authorization ran and returned 500 instead of falling back to the
generic create permission the way every other unrecognized type does.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@pmbrull
pmbrull requested a review from a team as a code owner August 17, 2026 07:42
Copilot AI lite review requested due to automatic review settings August 17, 2026 07:42
Copilot AI reviewed Aug 17, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

❌ 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 github-actions Bot added Ingestion safe to test Add this label to run secure Github workflows on PRs labels Aug 17, 2026
@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

⚠️ UI Checkstyle passed — lint findings in changed files

🔍 ESLint findings in this PR's files — 0 error(s), 49 warning(s)

Errors block the build. Warnings do not yet — they are rules whose backlog is still
being worked down, listed so this PR does not add to it. See docs/ui-code-quality-gate.md.

0 error(s), 49 warning(s) across 2 changed file(s).

Count Rule
44 react-hooks/exhaustive-deps
2 sonarjs/no-duplicate-string
1 openmetadata-imports/review-sequential-api-calls
1 sonarjs/cyclomatic-complexity
1 openmetadata-imports/no-internal-barrel-imports
All findings
Location Rule Message
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:371:8 react-hooks/exhaustive-deps React Hook useMemo has an unnecessary dependency: 'location.search'. Either exclude it or remove the dependency array. Outer scope values like 'location.search'
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:400:6 react-hooks/exhaustive-deps React Hook useMemo has an unnecessary dependency: 'serviceCategory'. Either exclude it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:426:5 react-hooks/exhaustive-deps React Hook useMemo has a missing dependency: 'navigate'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:438:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'setFilters'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:462:6 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'getEntityPermissionByFqn'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:468:6 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'navigate'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:508:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'handlePageChange', 'navigate', and 'setFilters'. Either include them or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:536:40 openmetadata-imports/review-sequential-api-calls Review these sequential API requests. If they are independent, start them together with Promise.all/Promise.allSettled; keep sequencing only when data-dependent
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:609:5 react-hooks/exhaustive-deps React Hook useCallback has an unnecessary dependency: 'ingestionPaging'. Either exclude it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:725:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'handlePagingChange'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:739:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'handlePagingChange'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:759:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'handlePagingChange'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:801:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'handlePagingChange'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:815:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'handlePagingChange'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:831:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'handlePagingChange'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:847:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'handlePagingChange'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:861:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'handlePagingChange'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:876:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'handlePagingChange'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:898:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'handleFilesPagingChange'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:923:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'handleSpreadsheetsPagingChange'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:987:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'handlePagingChange'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1030:6 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'navigate' and 'setFilters'. Either include them or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1052:6 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'followers', 'serviceDetails', and 't'. Either include them or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1077:6 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'serviceDetails' and 't'. Either include them or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1217:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 't'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1330:6 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'navigate'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1387:6 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'handleShowDeleted' and 'showDeleted'. Either include them or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1397:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'navigate'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1408:21 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 3 times.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1420:6 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 't'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1427:5 react-hooks/exhaustive-deps React Hook useMemo has an unnecessary dependency: 'serviceCategory'. Either exclude it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1456:8 react-hooks/exhaustive-deps React Hook useMemo has a missing dependency: 't'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1484:6 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'getOtherDetails'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1494:16 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 12 which is greater than 10 authorized.","cost":2,"secondaryLocations":[{"line":1494,"column":15,"endLine":1494,"endCol
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1529:6 react-hooks/exhaustive-deps React Hook useEffect has missing dependencies: 'fetchDashboardsDataModel', 'fileSearchValue', and 'spreadSheetSearchValue'. Either include them or remove the de
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1594:6 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'fetchServiceDetails'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1600:6 react-hooks/exhaustive-deps React Hook useEffect has missing dependencies: 'fetchServicePermission' and 'isOpenMetadataService'. Either include them or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1611:6 react-hooks/exhaustive-deps React Hook useEffect has missing dependencies: 'currentIngestionPage', 'getAllIngestionWorkflows', 'ingestionPagingCursor?.pageSize', 'isOpenMetadataService', a
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1625:6 react-hooks/exhaustive-deps React Hook useEffect has missing dependencies: 'collateAgentPagingCursor?.pageSize', 'fetchCollateAgentsList', and 'isCollateAIWidgetSupported'. Either include
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1629:6 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'fetchWorkflowInstanceStates'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1656:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'fetchCollateAgentsList'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1695:5 react-hooks/exhaustive-deps React Hook useMemo has an unnecessary dependency: 'ingestionPaging'. Either exclude it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1737:33 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 3 times.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1777:6 react-hooks/exhaustive-deps React Hook useMemo has a missing dependency: 't'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1959:6 react-hooks/exhaustive-deps React Hook useMemo has missing dependencies: 'pagingInfo', 'setFilters', and 't'. Either include them or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:2008:5 react-hooks/exhaustive-deps React Hook useMemo has a missing dependency: 'permissions'. Either include it or remove the dependency array.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:2008:25 react-hooks/exhaustive-deps React Hook useMemo has a complex expression in the dependency array. Extract it to a separate variable so it can be statically checked.
🟡 src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:2013:6 react-hooks/exhaustive-deps React Hook useCallback has an unnecessary dependency: 'serviceDetails.fullyQualifiedName'. Either exclude it or remove the dependency array.
🟡 src/rest/ingestionPipelineAPI.ts:32:1 openmetadata-imports/no-internal-barrel-imports Import the internal module directly instead of its index barrel so unrelated siblings do not enter the bundle graph.

Fix locally (fast - only checks files changed in this branch):

make ui-checkstyle-changed

@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 66%
 66.85% (79875/119477) 51.24% (48747/95126) 52.22% (14592/27938)

@pmbrull @claude
refactor(ingestion-pipelines): only record queued status where a user…
ec4fe63 
… watches for it

Drops the queued-status recording from RunAppImpl and DataContractRepository,
keeping it on the two paths whose runs render somewhere a user is waiting on
them: the manual trigger, AppResource (Applications page) and AutoPilot's
createAndRunIngestionPipeline (service Agents page).

The two removed paths are workflow-internal. `waitForCompletion` treats any
non-terminal status as still running, so the queued row bought their own
polling nothing, and recording it cost an upsert plus a search reindex on every
run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 17, 2026 08:02
Copilot AI reviewed Aug 17, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Aug 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit d7279e0ca6f5fd4ce0a59277843013d94a75f2cd in Playwright run 32021601919, attempt 2.

✅ 1091 passed · ❌ 0 failed · 🟡 1 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) 2h 23m 54s

⏱️ Max setup 3m 8s · max shard execution 20m 28s · max shard-job elapsed before upload 24m 17s · reporting 6s

🌐 215.13 requests/attempt · 2.40 app boots/UI scenario · 14.09% common-shard skew

Optimization targets still in progress:

  • Browser traffic was 215.13 requests per attempt (convergence target: fewer than 200).
  • Application boot ratio was 2.4 per UI scenario (2799 boots / 1164 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard chromium-01 172 0 0 0 0 0
✅ Shard chromium-02 165 0 0 0 0 0
✅ Shard chromium-03 161 0 0 0 0 0
✅ Shard chromium-04 188 0 0 0 0 0
✅ Shard chromium-05 181 0 0 0 0 0
✅ Shard data-asset-rules-01 61 0 0 0 0 0
✅ Shard domain-isolation-01 14 0 0 0 0 0
✅ Shard global-state-01 34 0 0 0 0 0
🟡 Shard ingestion-01 24 0 1 0 0 0
✅ Shard ingestion-02 49 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
🟡 1 flaky test(s) (passed on retry)
  • Pages/IngestionLogStreamLive.spec.ts › Live logs arrive over SSE while the agent runs, with no polling (shard ingestion-01, 1 retry)

📦 Download artifacts

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

chirag-madlani
chirag-madlani previously approved these changes Aug 17, 2026
View reviewed changes
@pmbrull @claude
fix(ingestion-pipelines): apply the stale-queued cutoff to the pipeli…
d7279e0 
…neStatuses field

The cutoff only ran in listPipelineStatus, but the Agents page does not read
that endpoint for its cards — it lists with `fields=owners,pipelineStatuses`,
and that field is built by getRecentPipelineStatuses / batchFetchRecentPipeline
Statuses, neither of which filtered. A run the orchestrator accepted but never
started therefore stayed `queued` forever on the one page this whole change is
about, since latestPipelineStatus reads the newest entry as the pipeline's
current state.

Moves the filter into toPipelineStatuses, the single conversion point both the
single-entity and the bulk read go through, so the timeout now holds wherever
the field is populated.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 17, 2026 10:44
Copilot AI reviewed Aug 17, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@gitar-bot

gitar-bot Bot commented Aug 17, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 1 resolved / 1 findings

Adds the agentType filter and records queued status directly using orchestrator run IDs to avoid polling, addressing the stale-queued timeout bypass finding. No issues found.

✅ 1 resolved
✅ Bug: Stale-queued timeout bypassed for pipelineStatuses field

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/IngestionPipelineRepository.java:147-150 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/IngestionPipelineRepository.java:232-246 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/IngestionPipelineRepository.java:718-719 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/IngestionPipelineRepository.java:775-789 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/IngestionPipelineRepository.java:714-728
The queuedStatusTimeoutSeconds filtering (dropStaleQueuedStatuses) is applied only in listPipelineStatus, but the pipelineStatuses field is populated by getRecentPipelineStatuses and batchFetchRecentPipelineStatuses (via setFields/fetchAndSetDefaultFields), which do not filter stale queued rows. The service Agents page loads pipelines with fields=owners,pipelineStatuses, so a run the orchestrator accepted but never started stays queued forever on the very page this change targets — exactly the trade-off the timeout was added to solve. Route these entity-field reads through withoutStaleQueuedStatuses too (or filter inside toPipelineStatuses) so the timeout applies consistently.

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

@sonarqubecloud

sonarqubecloud Bot commented Aug 17, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed for 'open-metadata-ui'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@sonarqubecloud

sonarqubecloud Bot commented Aug 17, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed for 'open-metadata-ingestion'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@pmbrull
pmbrull enabled auto-merge August 17, 2026 14:47
@pmbrull
pmbrull disabled auto-merge August 17, 2026 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gitar-bot gitar-bot[bot] gitar-bot[bot] left review comments
@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments
Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@chirag-madlani chirag-madlani chirag-madlani approved these changes
@karanh37 karanh37 Awaiting requested review from karanh37 karanh37 is a code owner
@ShaileshParmar11 ShaileshParmar11 Awaiting requested review from ShaileshParmar11 ShaileshParmar11 is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@pmbrull @chirag-madlani