Skip to content

Conversation

@MarcelGeo
Copy link
Collaborator

@MarcelGeo MarcelGeo commented Jan 19, 2026

  • add exception handling for clearing upload

Problem

In create project version controller wasn't handled ObjectDeletedError for upload row. Then, database versus data inconsistency occurred as no rollback was called there.

Future

Move locking of transaction to database using optimisitc locking (or similar), remove lockfile completely, ...

- add exception handling for clearing upload
Copy link
Contributor

Copilot AI left a 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 pull request adds error handling for ObjectDeletedError exceptions that can occur when an upload object is deleted by another process during project version creation. The changes add exception handling in two places: the controller's exception clause and within the Upload.clear() method itself.

Changes:

  • Added ObjectDeletedError to the list of exceptions caught during project version creation
  • Added try-except block in the Upload.clear() method to handle exceptions during cleanup
  • Added redundant upload.clear() call in exception handler (problematic)

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
server/mergin/sync/public_api_v2_controller.py Imports ObjectDeletedError, adds it to exception handling, and adds upload.clear() call in except block
server/mergin/sync/models.py Wraps Upload.clear() method logic in try-except block to handle cleanup failures

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@coveralls
Copy link

coveralls commented Jan 19, 2026

Pull Request Test Coverage Report for Build 21163477171

Details

  • 6 of 8 (75.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.02%) to 94.351%

Changes Missing Coverage Covered Lines Changed/Added Lines %
server/mergin/sync/models.py 4 6 66.67%
Totals Coverage Status
Change from base Build 21127635569: -0.02%
Covered Lines: 8100
Relevant Lines: 8585

💛 - Coveralls

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@MarcelGeo MarcelGeo merged commit 3e0aa0d into master Jan 20, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants