Repository navigation
feat(data-products): resolve input-port contractId and contract dataProduct on read path (#854) - #864
Merged
Merged
Conversation
…roduct on read (#854) Make product↔contract reference resolution symmetric on the read path, mirroring the existing output-port→contract treatment. Follow-up to #853; independent of the import stack (branched off development). Input-port contractId → contractName - Add contractName to the InputPort API model and resolve it in _load_product_with_tags (data_contract_repo.get by id), exactly mirroring the output-port block. A missing contract leaves contractName None; the UI falls back to the raw id. - data-product-details renders the input-port contract as a navigable Badge linking to /data-contracts/{contractId} (was a raw id label). Contract dataProduct → product link - Add dataProductName + dataProductId (resolved at query time) to DataContractRead. _build_contract_api_model resolves db_contract.data_product by id first, then a best-effort name match (the product repo has no get_by_name, so a bounded inline query is used). Missing/ambiguous → names stay None and the raw dataProduct is preserved. - data-contract-details adds a "Data Product" metadata row linking to /data-products/{dataProductId}, falling back to the raw value. No regression to the output-port→contract link or the get_products_by_contract "Linked Products" list (both untouched). Tests - New test_resolve_refs: input-port contractName resolved + graceful-missing; contract dataProduct resolved by id and by name + graceful-missing. Verified live (isolated 8140/3140): product read populates input-port contractName; contract detail shows a clickable Data Product link that navigates to the product. Session: claude -r 37a1420d-86af-4380-bef3-0b74c6f8b4b9
larsgeorge-db
added
the
stakeholder/fpiz
larsgeorge-db
marked this pull request as draft
September 24, 2026 12:57
larsgeorge-db
added a commit
that referenced
this pull request
Sep 27, 2026
…fpiz PRs Rebuilt test/combined-mcp-odcs-odps on the current development tip and merged every open PR labelled stakeholder/fpiz: - import stack #860/#862/#863 (feat/import-id-adopt) - #864 resolve-refs, #856 domain/tag dropdown scroll fix - search #865 (improve-search) + #867 (improve-search-phase2) Dropped a stray unused search_scoring import that the search-branch merge left in data_products.py (query_index is used). Registry imports; 43 search+import unit tests pass. Co-authored-by: Isaac <no-reply@databricks.com>
larsgeorge-db
marked this pull request as ready for review
October 7, 2026 08:23
mvkonchits-db
left a comment
mvkonchits-db
left a comment
Contributor
There was a problem hiding this comment.
Comment. Independent of the import stack and fine to land before #863, but please look at these first. Items marked (verified) I confirmed in the diff.
- (verified) Input-contract resolution does one
data_contract_repo.getper input port inside a loop. Iflist_productsinvokes it per product, as the review suggested, a 100-product list with five input ports adds about 500 queries. Please batch the lookups. - (verified) The resolver uses the raw repositories, so it does no visibility check. A readable contract can expose the name and id of a product the caller cannot open. Please confirm this is acceptable, or filter.
- (reported) The name fallback does not detect ambiguity despite the comment. Product names are non-unique and versioned, and
.first()has no ordering, so a legacydataProduct: "Orders"can resolve to an arbitrary version. - (low) The frontend renders any non-empty
contractIdas a link even when the backend found the reference dangling, so it leads to a 404.
Interaction with #863: that PR persists supplied UUIDs as primary keys, and this PR resolves references by id. Please land the id-squatting fix from #863 together with (or before) this.
5 tasks
…ack + dangling-link UI (#854 review) Addresses mvkonchits-db's review on #864: - Batched port.contractId → contractName in list_products (#854 N+1 review): new data_contract_repo.get_by_ids(ids) returns a {id: DataContractDb} map built with one IN(...) query; list_products now preloads it once and passes the map into _load_product_with_tags via a new preloaded_contracts kwarg. _resolve_port_contract uses the map when present and falls back to the per-port repo.get() for the single-entity callers (get_product, create, update, etc.) so the behavior is unchanged for them. A regression test monkey-patches data_contract_repo.get to confirm zero per-port calls during list_products — 500 queries for 100 products × 5 ports becomes 1. - Ambiguous dataProduct-name fallback (#854 review): the name-based product resolver used .first() with no ordering, so a non-unique product name (versions, drafts) could resolve to an arbitrary row and the UI would link to the wrong product. Now requires a unique match (limit=2 → len == 1); multiple matches leave (dataProductName, dataProductId) as None and the UI falls back to rendering the raw dataProduct string. - Dangling input/output port.contractId → render a non-clickable badge with a "contract no longer exists" tooltip instead of a navigable link that would 404 (#854 review). Only link when contractName was resolved. Tests: ambiguous-name graceful fallback + batched lookup regression. Co-authored-by: Isaac <no-reply@databricks.com>
mvkonchits-db
approved these changes
Oct 9, 2026
mvkonchits-db
left a comment
mvkonchits-db
left a comment
Contributor
There was a problem hiding this comment.
Re-reviewed c1a181fbb. Verified in the code:
- Batch lookup:
get_by_idsdoes oneINquery per page, andlist_productspasses the map down. The attribute names (input_ports,output_ports,contract_id) match the model. The new test asserts thatcontractNameresolves to{"C1","C2"}and thatdata_contract_repo.getis never called per port, so it cannot pass on an empty map. - Ambiguous name fallback: now requires a unique match (
limit(2), resolve only when exactly one), with a test. - Dangling contract ids render a non-clickable badge instead of a link that would 404.
- CI is green on the
developmentbase.
Not addressed, non-blocking and low: contract-to-product resolution still reads the raw repository without a visibility check. It only adds the product's name for a reference the contract already carries, so I am treating it as a follow-up. Cosmetic: get_by_ids landed under the "Override get_multi" comment.
Approving. It is safe to merge ahead of #863: ids are still server-generated on development until that lands.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #854.
Makes product↔contract reference resolution symmetric on the read path, mirroring the existing output-port→contract treatment. Two references that were stored but rendered as raw ids now resolve to navigable names.
Input-port
contractId→contractNamecontractNameto theInputPortAPI model; resolved in_load_product_with_tagsviadata_contract_repo.get(id), exactly mirroring the output-port block. Missing contract →contractNamestaysNone, UI falls back to the raw id.data-product-detailsrenders the input-port contract as a navigableBadgelinking to/data-contracts/{contractId}(was a raw id label).Contract
dataProduct→ product linkdataProductName+dataProductId(resolved at query time) toDataContractRead._build_contract_api_modelresolvesdata_productby id first, then a best-effort name match (the product repo has noget_by_name, so a bounded inline query is used). Missing/ambiguous → names stayNone, rawdataProductpreserved.data-contract-detailsadds a "Data Product" metadata row linking to/data-products/{dataProductId}, with graceful fallback.No regression to the output-port→contract link or the
get_products_by_contract"Linked Products" list (both untouched).Tests & verification
test_resolve_refs: input-port contractName resolved + graceful-missing; contract dataProduct resolved by id and by name + graceful-missing (5 tests).contractName; the contract detail page shows a clickable Data Product link that navigates to the product.This pull request and its description were written by Isaac.