Skip to content

Adapt to msgpack 1.2.0 (#3167) - #3180

Merged
IvoDD merged 1 commit into
6.18.2from
6-18-2-msgpack-cherry-pick
Jun 17, 2026
Merged

Adapt to msgpack 1.2.0 (#3167)#3180
IvoDD merged 1 commit into
6.18.2from
6-18-2-msgpack-cherry-pick

Conversation

@IvoDD

@IvoDD IvoDD commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Reference Issues/PRs

Fix test_deep_nesting_metastruct_size_over_limit and test_deep_nesting_metastruct_size_under_limit
Culprit: msgpack 1.2.0 has just released. It includes Raise DEFAULT_RECURSE_LIMIT from 511 to 1024 by Copilot · Pull Request #676 · msgpack/msgpack-python which changes DEFAULT_RECURSE_LIMIT from 511 to 1024 python 3.9 is unaffected as the new msgpack version has dropped the support

What does this implement or fix?

Hard-pin the arcticdb limit to 511 as msgpack <1.2.0 #### Any other comments?
msgpack <1.2.0 has the recurse limit set at 511. arcticdb imports the limit from msgpack to calculate the metastruct layer limit msgpack 1.2.0 has raised limit from 511 to 1024. Since reconstructing the metastruct from is done by recursion, increasing the no. of layers in the metastruct will make arcticdb hit the python recursion limit. #### Checklist

Checklist for code changes...
  • Have you updated the relevant docstrings, documentation and copyright notice?
  • Is this contribution tested against all ArcticDB's features?
  • Do all exceptions introduced raise appropriate error messages?
  • Are API changes highlighted in the PR description?
  • Is the PR labelled as enhancement or bug so it appears in autogenerated release notes?

Reference Issues/PRs

What does this implement or fix?

Any other comments?

Checklist

Checklist for code changes...
  • Have you updated the relevant docstrings, documentation and copyright notice?
  • Is this contribution tested against all ArcticDB's features?
  • Do all exceptions introduced raise appropriate error messages?
  • Are API changes highlighted in the PR description?
  • Is the PR labelled as enhancement or bug so it appears in autogenerated release notes?

#### Reference Issues/PRs
<!--Example: Fixes #1234. See also #3456.-->
Fix `test_deep_nesting_metastruct_size_over_limit` and
`test_deep_nesting_metastruct_size_under_limit`
Culprit: `msgpack 1.2.0` has just released. It includes [Raise
DEFAULT_RECURSE_LIMIT from 511 to 1024 by Copilot · Pull Request #676 ·
msgpack/msgpack-python](msgpack/msgpack-python#676)
which changes `DEFAULT_RECURSE_LIMIT` from 511 to 1024
`python 3.9` is unaffected as the new msgpack version has dropped the
support

#### What does this implement or fix?
Hard-pin the arcticdb limit to `511` as msgpack <1.2.0
#### Any other comments?
msgpack <1.2.0 has the recurse limit set at 511. arcticdb imports the
limit from msgpack to calculate the metastruct layer limit
msgpack 1.2.0 has raised limit from 511 to 1024. Since reconstructing
the metastruct from is done by recursion, increasing the no. of layers
in the metastruct will make arcticdb hit the python recursion limit.
#### Checklist

<details>
  <summary>
   Checklist for code changes...
  </summary>
 
- [ ] Have you updated the relevant docstrings, documentation and
copyright notice?
- [ ] Is this contribution tested against [all ArcticDB's
features](../docs/mkdocs/docs/technical/contributing.md)?
- [ ] Do all exceptions introduced raise appropriate [error
messages](https://docs.arcticdb.io/error_messages/)?
 - [ ] Are API changes highlighted in the PR description?
- [ ] Is the PR labelled as enhancement or bug so it appears in
autogenerated release notes?
</details>

<!--
Thanks for contributing a Pull Request to ArcticDB! Please ensure you
have taken a look at:
- ArcticDB's Code of Conduct:
https://github.com/man-group/ArcticDB/blob/master/CODE_OF_CONDUCT.md
- ArcticDB's Contribution Licensing:
https://github.com/man-group/ArcticDB/blob/master/docs/mkdocs/docs/technical/contributing.md#contribution-licensing
-->
@IvoDD
IvoDD merged commit 7c4788b into 6.18.2 Jun 17, 2026
11 checks passed
@IvoDD
IvoDD deleted the 6-18-2-msgpack-cherry-pick branch June 17, 2026 07:57
@claude

claude Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

ArcticDB Code Review Summary

The change correctly hard-pins DEFAULT_RECURSE_LIMIT to 511 to insulate ArcticDB from msgpack 1.2.0 raising its own constant to 1024. The public name is preserved (only the msgpack import is aliased), the write-cap guard and tests stay consistent, and the boundary arithmetic checks out (over-limit at 256 nesting levels → message reports 255). The explanatory comment is valuable and accurate. The logic is sound and well-tested.

PR Title & Description

  • No bug/enhancement label is set. This is a behavioural fix (on msgpack 1.2.0 it caps recursive-normalizer writes at 255 levels rather than silently allowing writes the reader cannot reconstruct), so it should be labelled and should not be marked no-release-notes.

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.

2 participants