Skip to content

fix(ontology): bound the concept binding lookup and declare property-chain members in the import IT - #36644

Merged
manerow merged 2 commits into
mainfrom
fix/ontology-main-red-its
Oct 30, 2026
Merged

manerow merged 2 commits into
mainfrom
fix/ontology-main-red-its

Conversation

@sonika-shah

Copy link
Copy Markdown
Collaborator

Describe your changes:

Main has been red on three ontology ITs since #35991 and #35996 merged, in every lane, so every merge-queue build fails them and the queue cannot move.

  • OntologyConceptBindingCleanupIT (2 tests): the scan probe requires a LIMIT on every ontology_concept_binding read, but OntologyConceptBindingDAO.find had none. It is a primary-key lookup on (conceptId, assetId), so LIMIT 1 changes no result. Same pattern as findPropertyEdgeAxiomPosition ... LIMIT 1.
  • NativeOntologyImportListRulesIT.aFilePreviewAcceptsAlsoAppliesWhenItsListsAreUsable: the CSV built a property chain over two properties it never declared, which apply rejects (Declare or import the referenced property, a rule since Fixes 35671: Add native ontologies and governed releases #35672). The CSV now declares both chain members; the new-property count goes from 2 to 4.

Both tests landed with these bugs; PR CI does not run ITs, so they first ran on main.

Tests:

  • Local, MySQL + Elasticsearch: OntologyConceptBindingCleanupIT 2/2 and NativeOntologyImportListRulesIT 4/4 pass (Tests run: 6, Failures: 0, Errors: 0), BUILD SUCCESS.

Type of change:

  • Bug fix

OntologyConceptBindingCleanupIT asserts that every binding scan carries a
LIMIT, but OntologyConceptBindingDAO.find (a primary-key lookup) had none.
Add LIMIT 1; the key is (conceptId, assetId), so results are unchanged.

NativeOntologyImportListRulesIT built a property chain over two properties
the file never declared, which apply rejects ("Declare or import the
referenced property"). Declare both chain members in the CSV.
@sonika-shah
sonika-shah requested a review from a team as a code owner October 30, 2026 05:51
@sonika-shah
sonika-shah requested a review from pmbrull October 30, 2026 05:51
@github-actions

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.

@sonika-shah sonika-shah added safe to test Add this label to run secure Github workflows on PRs backend labels Oct 30, 2026
@sonika-shah sonika-shah changed the title test(ontology): fix the three ontology ITs that keep main red fix(ontology): bound the concept binding lookup and declare property-chain members in the import IT Oct 30, 2026
@sonika-shah
sonika-shah removed the request for review from pmbrull October 30, 2026 06:31
@gitar-bot

gitar-bot Bot commented Oct 30, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

🟡 Medium risk · Adding LIMIT 1 to the ontology binding lookup could conceal duplicate matching rows; the other change only adjusts integration-test data.

Adds LIMIT 1 to the OntologyConceptBindingDAO.find primary-key lookup and declares both property chain members in the NativeOntologyImportListRulesIT CSV, fixing three ontology ITs that were failing on main. No issues found.

Verification

Rules No rules evaluated

Functional validation Not enabled

Set up

Auto-approval Not enabled

Set up

For more verification details, comment gitar display:verbose.

Options

Display: compact → Showing compact verification results.

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

@mohityadav766
mohityadav766 added this pull request to the merge queue Oct 30, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🚦 Removed from the merge queue — failed_checks (2026-10-30T08:07:47Z)

Blocked the queue: playwright-summary

Failed Playwright tests:

  • Features/OntologyNativeOperations.spec.ts › retries failed review submission across tabs using the same captured release
  • Features/OntologyNativeTreeShell.spec.ts › keeps both parent placements, one selection and all header controls at 924 pixels
  • Features/OntologyReview.spec.ts › manages reviewers with reload persistence and locks settings during an open review
  • Flow/ProductTour.spec.ts › walks the Classic pages on recorded data and returns to Home

@manerow
manerow merged commit 9290559 into main Oct 30, 2026
73 of 75 checks passed
@manerow
manerow deleted the fix/ontology-main-red-its branch October 30, 2026 09:22
manerow pushed a commit that referenced this pull request Oct 30, 2026
OntologyConceptBindingCleanupIT asserts that every binding scan carries a
LIMIT, but OntologyConceptBindingDAO.find (a primary-key lookup) had none.
Add LIMIT 1; the key is (conceptId, assetId), so results are unchanged.

NativeOntologyImportListRulesIT built a property chain over two properties
the file never declared, which apply rejects ("Declare or import the
referenced property"). Declare both chain members in the CSV.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend 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.

3 participants