Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CBOR serializer (using cbor2 package) #191

Merged

Conversation

brendan-simon-indt
Copy link
Contributor

Add CBOR serializer support, using the cbor2 package.
CBOR protocol is similar to msgpack, however CBOR has a cleaner design, is standardized and preferred over msgpack by some.

I modelled the implementation based on the msgpack implementation.

I arbitrarily chose cbor2 5.2.0 has the minimum version requirement, as this was the first release to make it into a Debian distribution as a pre-built package (Debian 11 Bullseye). It is likely that earlier cbor2 versions would also work.

queue.put({"foo": 1, "bar": 2})
self.assertEqual(queue.total, 1)
queue.put({"bar": 2, "foo": 1})
self.assertEqual(queue.total, 1)
Copy link
Owner

Choose a reason for hiding this comment

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

Better to verify the content in the queue as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

None of the other tests (pickle, msgpack, json) verify the content in the queue. I presume this is because this particular test is testing that duplicate items are only queued once for UniqueQ. I also presume that verifiying the content is handled in test_queue.py

Copy link
Owner

Choose a reason for hiding this comment

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

You're right, we have more test to cover serialization scenario for file based queues, maybe we can enhance the coverage next time for sqlite base queues

@peter-wangxu
Copy link
Owner

@brendan-simon-indt looks fine, please address the comment in code and pep8 issue in CI

./persistqueue\serializers\cbor2.py:29:1: E302 expected 2 blank lines, found 1

queue.put({"foo": 1, "bar": 2})
self.assertEqual(queue.total, 1)
queue.put({"bar": 2, "foo": 1})
self.assertEqual(queue.total, 1)
Copy link
Owner

Choose a reason for hiding this comment

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

You're right, we have more test to cover serialization scenario for file based queues, maybe we can enhance the coverage next time for sqlite base queues

@peter-wangxu peter-wangxu merged commit 6f817ec into peter-wangxu:master Feb 5, 2023
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