Accept fullyQualifiedName on the ingestion pipeline log endpoints#30187
Accept fullyQualifiedName on the ingestion pipeline log endpoints#30187Khairajani wants to merge 5 commits into
Conversation
The two log endpoints, GET /logs/{id}/last and /logs/{id}/last/download,
accepted only a pipeline Id (UUID). They now accept either the Id or the
pipeline's fullyQualifiedName. The Id form is unchanged and remains fully
supported; the fqn is purely additive.
Logs are keyed by the pipeline's fully-qualified name, so a caller holding
the fqn had to resolve it to an Id before it could ask for logs. Accepting
the fqn removes that extra lookup and keeps a pipeline's log URL stable for
its lifetime. Dispatch is unambiguous: a pipeline Id is always a UUID, an
fqn never is, so the path segment is parsed as a UUID first and treated as
an fqn only when that fails.
The UI now fetches and downloads logs by fqn. A response keyed by fqn has no
pipeline type to select a *_task field, so it carries the log text under a
generic `logs` key; the two extractors prefer that key and fall back to the
type-specific field, leaving by-id responses handled exactly as before.
✅ PR checks passedThe linked issue has a description and all required Shipping project fields set. Thanks! |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
Khairajani
added
the
safe to test
Jest test CoverageUI tests summary
|
Guards that both log endpoints (getLastIngestionLogs, downloadLastIngestionLogs) expose their id path segment as a String with schema type=string, which is what lets them accept either a pipeline Id (UUID) or a fullyQualifiedName. A UUID-typed parameter would reject an fqn at the framework boundary before the resource could dispatch on it, so this locks in the backward-compatible widening.
The paginated log fetch fires by fullyQualifiedName, but the enable gate and reset key still keyed on the pipeline Id. A pipeline whose details carry an Id but no fqn would enable the hook and request `/logs//last` with an empty segment. Gate and key on the fqn instead, so the hook waits for the identifier it actually uses.
|
Good catch on the P2 — fixed in b26653b. The paginated fetch runs by fqn, but the |
…ting Both log endpoints advertised "by `Id`" in their @operation description and a "Logs for instance {id}" 404, though the path segment now accepts a fullyQualifiedName too. Update the descriptions to "`Id` or `fullyQualifiedName`" and reword the 404 so the generated OpenAPI is accurate. Apply eslint/prettier to useEntityLogs.test.ts.
|
Addressed in af959be:
|
Code Review ✅ Approved 2 resolved / 2 findingsEnables ingestion pipeline log endpoints to accept fully qualified names alongside UUIDs, resolving the need for redundant ID lookups. Backend test coverage and documentation have been updated to reflect the new ID-or-FQN contract. ✅ 2 resolved✅ Bug: No backend test for UUID-vs-fqn log endpoint dispatch
✅ Quality: Download endpoint docs still say logs are fetched by Id
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
|
🔴 Playwright Results — 1 failure(s), 27 flaky✅ 4542 passed · ❌ 1 failed · 🟡 27 flaky · ⏭️ 96 skipped
Genuine Failures (failed on all attempts)❌
|
1 participant
Closes #30197
What
GET /services/ingestionPipelines/logs/{id}/lastand/logs/{id}/last/downloadaccepted only a pipeline Id (UUID). They now accept either the Id or the pipeline's fullyQualifiedName.The Id form is unchanged and remains fully supported — the fqn is purely additive, so this is backward compatible.
Why
Logs are keyed by the pipeline's fully-qualified name. A caller that already holds the fqn had to first resolve it to an Id just to ask for logs, which is an extra lookup for an identifier the log store never uses. Accepting the fqn removes that hop and keeps a pipeline's log URL stable across its lifetime.
How
A pipeline Id is always a UUID and an fqn never is, so dispatch is unambiguous: the path segment is parsed as a UUID first and treated as an fqn only when that parse fails.
Both endpoints keep
Include.NON_DELETEDand the samefieldsset, so authorization and visibility are unchanged on both branches.UI
The log viewer and the agents view now fetch and download by fqn.
A response keyed by fqn has no pipeline type available to select a
*_taskfield, so it carries the log text under a genericlogskey. The two extractors (getLogsFromResponse,getLogTaskFieldForType) prefer that key and fall back to the type-specific field — by-id responses are handled exactly as before.Tests
logskey inLogsUtils.test.tsandagentsDataMapper.test.ts, including precedence over the*_taskfield.useEntityLogs.test.tsupdated for the by-fqn fetch/download.Greptile Summary
This PR lets ingestion pipeline log endpoints accept either a UUID or a fully qualified name. The main changes are:
Confidence Score: 5/5
This looks safe to merge.
Important Files Changed
Comments Outside Diff (1)
openmetadata-ui/src/main/resources/ui/src/hooks/useEntityLogs.ts, line 121 (link)The request now uses
ingestionFqn, but this gate still checksingestionId. If pipeline details contain an ID without a fully qualified name, the hook is enabled and requests/logs//last, producing a failed request instead of waiting for the identifier it actually needs.Context Used: CLAUDE.md (source)
Reviews (5): Last reviewed commit: "Align log endpoint docs with the id-or-f..." | Re-trigger Greptile
Context used: