Conversation
Code Review ✅ ApprovedFixes the field setter regression by restoring correct state handling for export operations. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
There was a problem hiding this comment.
Pull request overview
Fixes a regression in the entity history-by-timestamp flow where returned historical entities were missing expected fields (e.g., service) by re-applying field population logic after deserializing history snapshots.
Changes:
- Update
listEntityHistoryByTimestamp(...)to callsetFieldsInBulk(...)on deserialized history entities. - Add an integration test to ensure the
/historyresponse for Databases includes a non-nullservicefield across returned versions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java | Re-hydrates fields for entities returned by the history-by-timestamp query. |
| openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/DatabaseResourceIT.java | Adds coverage to assert /databases/.../history returns versions that include service. |
|
|
||
| List<T> entities = JsonUtils.readObjects(jsons, getEntityClass()); | ||
| hydrateHistoryEntities(entities); | ||
| setFieldsInBulk(putFields, entities); |
There was a problem hiding this comment.
Calling setFieldsInBulk(putFields, entities) here will re-hydrate relationship-backed fields from the current DB state (owners/tags/domains/etc.) and can overwrite the historical snapshot data returned by the /history-by-timestamp query. It also increases the cost of the endpoint significantly compared to returning the stored snapshots.
Consider switching to a minimal history hydration strategy (e.g., only populate required default references like service when missing, without fetching other relationship fields), or restoring the previous lightweight hook-based approach so that /history continues to reflect the stored version snapshots.
| setFieldsInBulk(putFields, entities); |
|
|
||
| List<T> entities = JsonUtils.readObjects(jsons, getEntityClass()); | ||
| hydrateHistoryEntities(entities); | ||
| setFieldsInBulk(putFields, entities); |
There was a problem hiding this comment.
This change removes the only call site of hydrateHistoryEntities(...), leaving the method (and its Javadoc describing intentionally lightweight history hydration) effectively dead/inaccurate. Either call the hook again (possibly after deserialization and before pagination cursor computation) or remove/update the hook and documentation to match the new behavior.
| setFieldsInBulk(putFields, entities); | |
| setFieldsInBulk(putFields, entities); | |
| hydrateHistoryEntities(entities); |
|
🔴 Playwright Results — 1 failure(s), 18 flaky✅ 3662 passed · ❌ 1 failed · 🟡 18 flaky · ⏭️ 89 skipped
Genuine Failures (failed on all attempts)❌
|



Describe your changes:
Fix regression introduced in entity history where fields are not set anymore
Type of change:
Checklist:
Fixes <issue-number>: <short explanation>