-
Notifications
You must be signed in to change notification settings - Fork 228
Fix PullMerge() failure #8285
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Fix PullMerge() failure #8285
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR addresses a failure in PullMerge()
by adding validation and tooling around changeset application.
- Adds a standalone test to verify that applying a schema changeset with no local edits does not create pending transactions.
- Introduces
exportAsJson
inSqliteChangesetReader
to help debug and inspect changesets. - Updates the change metadata file to document this fix.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
File | Description |
---|---|
core/backend/src/test/standalone/ApplyChangeset.test.ts | New test verifying no local changes after pull merge |
core/backend/src/SqliteChangesetReader.ts | Added exportAsJson helper for dumping changesets |
common/changes/@itwin/core-backend/affank-fix-pullmerge_2025-06-27-11-56.json | Metadata entry for this PullMerge fix |
Comments suppressed due to low confidence (1)
core/backend/src/SqliteChangesetReader.ts:434
- The new
exportAsJson
utility is untested; consider adding unit tests to validate JSON structure, filtering logic, hashing of blobs, and indentation.
public exportAsJson(
common/changes/@itwin/core-backend/affank-fix-pullmerge_2025-06-27-11-56.json
Outdated
Show resolved
Hide resolved
common/changes/@itwin/core-backend/affank-fix-pullmerge_2025-06-27-11-56.json
Show resolved
Hide resolved
b1.close(); | ||
b2.close(); | ||
|
||
HubMock.shutdown(); |
Copilot
AI
Jun 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test starts IModelHost
but never shuts it down; consider adding an after
hook to call await IModelHost.shutdown()
and ensure clean teardown.
Copilot uses AI. Check for mistakes.
…6-27-11-56.json Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ffank/fix-pullmerge
imodel-native: iTwin/imodel-native#1162
Issue Summary
Initial Changeset Push:
OpenSite+ modified the GCS and pushed a changeset.
Schema Import by Civil Connector:
Civil Connector imported a schema and made additional GCS changes.
PullMerge Failure in OpenSite+:
OpenSite+ (with no local changes) attempted a
PullMerge()
, which failed due to the following sequence:TxnManager
disables tracking duringapplyChangeset()
, but afterward, it recomputesec_cache_*
tables, which are tracked.pullMergeEnd()
callssaveChanges()
, creating a transaction.ec_cache_*
changes.ec_cache_*
tables referenceec_class
via foreign keys. Reverting the cache tables leads to missing parentec_class
entries.Solution
Exclude
ec_cache_*
from Tracking:These tables are derived and can be safely recomputed, so they should not be tracked.
Disable
NOFKCONSTRAINT
Flag for Schema Changesets:Since
ec_cache_*
tables have FK relationships toec_class
andec_table
, cascading deletes must be allowed. This ensures that when primary tables are modified, related cache entries are also cleaned up properly.Additional Safeguard
PullMergeEnd()
, as ideally, there should be none.