Skip to content

fix: TypeError when restoring published containers#551

Merged
kdmccormick merged 1 commit intomainfrom
kdmccormick/fix-version-num
Apr 20, 2026
Merged

fix: TypeError when restoring published containers#551
kdmccormick merged 1 commit intomainfrom
kdmccormick/fix-version-num

Conversation

@kdmccormick
Copy link
Copy Markdown
Member

@kdmccormick kdmccormick commented Apr 19, 2026

Description

The original version of this code depended upon version_num being popped out
of valid_published before valid_published got expanded. A recent change
changed the argument order, leading to:

TypeError: create_next_container_version() got an unexpected keyword argument
'version_num'

whenever restoring an archive with any published containers. This commit
restores the original code's order of operations, but a bit more explicitly.

Bumps from 0.39.2 to 0.40.0 (it would be 0.39.3, but the previous
commit had breaking changes and neglected to bump the minor version)

Testing

On openedx-platform master and openedx-core==v0.39.0, back up any library with at least one published component.

Try to restore it. It'll fail. You should see a TypeError in the logs.

Cherry-pick this commit on top of openedx-core v0.39.0. Try again to restore the archive. It should succeed.

@kdmccormick kdmccormick force-pushed the kdmccormick/fix-version-num branch from aea9cd2 to 8430cf3 Compare April 19, 2026 20:57
The original version of this code depended upon `version_num` being popped out
of `valid_published` before `valid_published` got expanded. A recent change
changed the argument order, leading to:

  TypeError: create_next_container_version() got an unexpected keyword argument
  'version_num'

whenever restoring an archive with any published containers. This commit
restores the original code's order of operations, but a bit more explicitly.

Bumps from 0.39.2 to 0.40.0 (it would be 0.39.3, but the previous
commit had breaking changes and neglected to bump the minor version)
@kdmccormick kdmccormick force-pushed the kdmccormick/fix-version-num branch from 8430cf3 to d0281d1 Compare April 19, 2026 21:00
@kdmccormick kdmccormick marked this pull request as ready for review April 19, 2026 21:01
assert container.created_by.username == "lp_user"
# The unit has been published. The other two containers haven't been.
# It's important that we test with at least one published container in order to
# fully cover _create_container in zipper.py.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Suggested change
# fully cover _create_container in zipper.py.
# fully cover _save_container in zipper.py.

(I typo'd the function name)

@kdmccormick kdmccormick changed the title fix: Don't pass version_num to create_next_container_version fix: TypeError when restoring published containers Apr 19, 2026
@kdmccormick
Copy link
Copy Markdown
Member Author

@bradenmacdonald @ormsbee a quick fix to a bug on openedx-core main, could you take a look?

@kdmccormick kdmccormick force-pushed the kdmccormick/fix-version-num branch from 2de714b to d0281d1 Compare April 19, 2026 21:16
Copy link
Copy Markdown
Contributor

@ormsbee ormsbee left a comment

Choose a reason for hiding this comment

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

Code looks fine. Just a comment about the version.


# The version for the entire repository
__version__ = "0.39.2"
__version__ = "0.40.0"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since it's a bugfix, shouldn't this be 0.39.3?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The collection_code change is also released by this, which makes it a breaking release

Copy link
Copy Markdown
Contributor

@bradenmacdonald bradenmacdonald left a comment

Choose a reason for hiding this comment

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

Thanks! Yeah the current backup/restore code doesn't have enough test coverage.

@kdmccormick kdmccormick merged commit cdcf40c into main Apr 20, 2026
11 checks passed
@kdmccormick kdmccormick deleted the kdmccormick/fix-version-num branch April 20, 2026 15:36
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.

3 participants