msgpack>0.5, check instead of assert, test msgpack padding, typos and…

… style fixes

environment_unix.yml

@@ -66,7 +66,8 @@ dependencies:
   - werkzeug
   - moto
   - mock
-  - msgpack-python
+  # msgpack 0.5.0 is required for strict_types argument, needed for correct pickling fallback
+  - msgpack-python >= 0.5.0
   # Pinned to avoid test disruption (in phase with the pin in setup.cfg)
   # See: https://github.com/man-group/ArcticDB/pull/291
   - hypothesis < 6.73

python/arcticdb/_msgpack_compat.py

@@ -5,7 +5,7 @@
 This module implements a backwards compatible version of msgpack functions.
 """
 import msgpack
-from arcticdb.log import version as log
+from arcticdb.preconditions import check
 from arcticdb.exceptions import ArcticNativeException
 
 ExtType = msgpack.ExtType
@@ -20,7 +20,6 @@ def _check_valid_msgpack():
         packer_module in ("msgpack._packer", "msgpack.fallback", "msgpack._cmsgpack")
     ):
         return
-    log.info("Unsupported msgpack variant, got: {}, {}".format(pack_module, packer_module))
     raise ArcticNativeException("Unsupported msgpack variant, got: {}, {}".format(pack_module, packer_module))
 
 
@@ -44,7 +43,7 @@ def padded_packb(obj, **kwargs):
     nbytes = packer.getbuffer().nbytes
     pad = -nbytes % 8 # next multiple of 8 bytes
     [packer.pack(None) for _ in range(pad)] # None is packed as single byte b`\xc0`
-    assert packer.getbuffer().nbytes % 8 == 0
+    check(packer.getbuffer().nbytes % 8 == 0, 'Error in ArcticDB padded_packb. Padding failed. nbytes={}', packer.getbuffer().nbytes)
     return packer.bytes(), nbytes
 
 

python/arcticdb/version_store/_normalization.py

@@ -1225,10 +1225,9 @@ def normalize_metadata(d):
     packed = _msgpack_metadata._msgpack_packb(d)
     size = len(packed)
     if size > _MAX_USER_DEFINED_META:
-        raise ArcticDbNotYetImplemented("User defined metadata cannot exceed {}B".format(_MAX_USER_DEFINED_META))
+        raise ArcticDbNotYetImplemented(f'User defined metadata cannot exceed {_MAX_USER_DEFINED_META}B')
     if size > _WARN_USER_DEFINED_META:
-        log.warn('User defined metadata is above warning size ({0}B), metadata cannot exceed {1}B.  Current size: {2}B.'\
-                 .format(_WARN_USER_DEFINED_META, _MAX_USER_DEFINED_META, size))
+        log.warn(f'User defined metadata is above warning size ({_WARN_USER_DEFINED_META}B), metadata cannot exceed {_MAX_USER_DEFINED_META}B.  Current size: {size}B.')
 
     udm = UserDefinedMetadata()
     udm.inline_payload = packed

python/tests/unit/arcticdb/test_msgpack_compact.py

@@ -19,7 +19,7 @@ def test_packb_raises_on_tuple():
 def test_padded_packb_small():
     packed, nbytes = padded_packb(1)
     assert len(packed) % 8 == 0
-    assert len(packed) > nbytes
+    assert len(packed) >= nbytes
     assert len(packed) - nbytes < 8
     assert packed[:nbytes] == msgpack.packb(1, use_bin_type=True, strict_types=True)
 
@@ -34,14 +34,21 @@ def test_padded_packb_list():
 
 
 def test_padded_packb_string():
-    aas = 'A' * 1_000_005 # not dividisble by 8
+    aas = 'A' * 1_000_005 # not divisible by 8
     packed, nbytes = padded_packb(aas)
     assert len(packed) % 8 == 0
     assert len(packed) >= nbytes
     assert len(packed) - nbytes < 8
     assert packed[:nbytes] == msgpack.packb(aas, use_bin_type=True, strict_types=True)
 
 
+def test_padded_packb_padding():
+    # padded_packb behaviour relies on 1 byte for None assumption from msgpack spec
+    packed, nbytes = padded_packb(None)
+    assert nbytes == 1  # 1 byte of content
+    assert packed == b'\xc0\xc0\xc0\xc0\xc0\xc0\xc0\xc0' # 7 bytes of padding, 8 total
+
+
 def test_unpackb():
     # serializes without strict_types
     packed = msgpack.packb({(1, 2): "a"})

python/tests/unit/arcticdb/version_store/test_normalization.py

@@ -369,7 +369,7 @@ def test_ndarray_arbitrary_shape():
 
 
 def test_dict_with_tuples():
-    # This has to be pickeld because msgpack doesn't differentiate between tuples and lists
+    # This has to be pickled because msgpack doesn't differentiate between tuples and lists
     data = {(1, 2): [1, 24, 55]}
     norm = CompositeNormalizer(use_norm_failure_handler_known_types=False, fallback_normalizer=test_msgpack_normalizer)
     df, norm_meta = norm.normalize(data)

setup.cfg

@@ -39,7 +39,8 @@ install_requires =
     attrs
     dataclasses ; python_version < '3.7'
     protobuf >=3.5.0.post1, < 5 # Per https://github.com/grpc/grpc/blob/v1.45.3/requirements.txt
-    msgpack >=0.5.0 # 0.5.0 is needed for strict_types argument, which is required for our pickling behaviour
+    msgpack >=0.5.0 # msgpack 0.5.0 is required for strict_types argument, needed for correct pickling fallback
+
     pyyaml
     packaging