Skip to content

fix(media-files): Skip sync when draft files doesn't have a bucket#326

Merged
kpsherva merged 1 commit intoinveniosoftware:masterfrom
sakshamarora1:fix/media_files_sync
Mar 10, 2026
Merged

fix(media-files): Skip sync when draft files doesn't have a bucket#326
kpsherva merged 1 commit intoinveniosoftware:masterfrom
sakshamarora1:fix/media_files_sync

Conversation

@sakshamarora1
Copy link
Contributor

@sakshamarora1 sakshamarora1 commented Mar 9, 2026

  • When publishing an edit, _publish_edit always calls bucket.sync(), however the method assumes that the draft files manager has a bucket already initialized but if the media files flag is disabled and no files were ever uploaded to the draft, the bucket is not created and the sync errors out.

Note

  • The problem only became visible on python3.14 and only on the CI runs, I was not able to figure out why exactly.
  • I checked the dependencies differences between 3.9/3.11 version and 3.14 and they were 2-3 unrelated small packages
  • I also checked the dependency changes between when 3.14 passed 2 weeks ago vs failing now and these are the changes in versions:
certifi 2026.1.4 -> 2026.2.25
SQLAlchemy 2.0.46 -> 2.0.48
redis 7.2.0 -> 7.3.0
maxminddb 3.0.0 -> 3.1.1
invenio-db 2.2.1 -> 2.3.0
invenio-administration 5.0.0 -> 5.1.0
invenio-stats 6.1.0 -> 6.1.2
ua-parser-builtins 202602 -> 202603
wrapt 2.1.1 -> 2.1.2
platformdirs 4.9.2 -> 4.9.4
isort 8.0.0 -> 8.0.1
black 26.1.0 -> 26.3.0
ipython 9.10.0 -> 9.11.0
imagesize 1.4.1 -> 2.0.0
charset-normalizer 3.4.4 -> 3.4.5
pytest-github-actions-annotate-failures 0.3.0 -> 0.4.0
  • When I run locally the test suite then everything passes (but if it fails for someone else, please mention here to have more context!)

  * When publishing an edit, `_publish_edit` always calls bucket.sync(), however the method assumes that the draft files manager has a bucket already initialized but if the media files flag is disabled and no files were ever uploaded to the draft, the bucket is not created and the sync errors out.
@sakshamarora1 sakshamarora1 force-pushed the fix/media_files_sync branch from fdb7a37 to d2550cc Compare March 9, 2026 15:00
@kpsherva kpsherva moved this to In review 🔍 in Sprint Q1/2026 Mar 9, 2026
Comment on lines +127 to +132
# If the draft files manager has a bucket associated with it, then we need to sync to
# propagate the changes to the record files manager regardless of whether files are enabled.
# The sync() method assumes the source file manager has a bucket, so we need to check for it.
draft_bucket = self.get_record_bucket(draft)
if draft_bucket:
record_files.sync(draft_files, delete_extras=True)
Copy link
Member

@ptamarit ptamarit Mar 10, 2026

Choose a reason for hiding this comment

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

Can this bug be covered be covered by a test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test_multiple_edit test function is is covering this. It has media files disabled. it's the test that was failing before.

@kpsherva kpsherva merged commit d31f8dc into inveniosoftware:master Mar 10, 2026
3 checks passed
@github-project-automation github-project-automation bot moved this from In review 🔍 to To release 🤖 in Sprint Q1/2026 Mar 10, 2026
@kpsherva kpsherva moved this from To release 🤖 to Released ✔️ in Sprint Q1/2026 Mar 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Released ✔️

Development

Successfully merging this pull request may close these issues.

3 participants