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

Implement customizable serializer #214

Merged
merged 1 commit into from
Oct 10, 2023
Merged

Conversation

asvetlov
Copy link
Contributor

Pre-requirement for taskiq-python/taskiq-pipelines#13

The main idea is:

  1. JSONFormatter exists for the backward compatibility.
  2. ProxyFormatter is used by default, which operates over broker.serializer API
  3. The default serializer is JSONSerializer which is a thin wrapper around plain json.dumps() and json.loads()
  4. TaskiqSerializer base class works with bytes, not str. The broker message type is a bytes anyway, plus formats like msgpack and cbor work with bytes anyway.

I can implement cbor and msgpack serializers as well if you are ok with optional dependencies (say, cbor2 is required only if a user explicitly imports taskiq.serializers.cbor_serializer:CBORSerializer.

@asvetlov asvetlov force-pushed the serializer branch 2 times, most recently from 7ba6e00 to b56b9ce Compare October 10, 2023 17:07
Copy link
Member

@s3rius s3rius left a comment

Choose a reason for hiding this comment

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

Now I see the point crystally clear. These changes will create a simpler API for defining your own serializers. Thank you very much.

I'm totally ok with optional dependencies. In the next iteration I was planning to add a lot of optional depndencies to make installation process easier.

@s3rius
Copy link
Member

s3rius commented Oct 10, 2023

To fix mypy please consider running poetry update.

To fix failing tests, make them async and mark with pytest.mark.anyio. This problem happens because we create a semaphore in InmemoryBroker. This works in py3.10 and above, but doesn't work for older versions. We will fix it.

@asvetlov
Copy link
Contributor Author

asvetlov commented Oct 10, 2023

tests should be green now, at least locally I don't see any problem

@codecov-commenter
Copy link

Codecov Report

Merging #214 (c198624) into develop (e66f3aa) will increase coverage by 5.65%.
Report is 131 commits behind head on develop.
The diff coverage is 73.28%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@             Coverage Diff             @@
##           develop     #214      +/-   ##
===========================================
+ Coverage    67.62%   73.27%   +5.65%     
===========================================
  Files           37       51      +14     
  Lines          942     1553     +611     
===========================================
+ Hits           637     1138     +501     
- Misses         305      415     +110     
Files Coverage Δ
taskiq/__init__.py 100.00% <100.00%> (ø)
taskiq/abc/formatter.py 100.00% <100.00%> (ø)
taskiq/abc/middleware.py 100.00% <ø> (ø)
taskiq/abc/result_backend.py 100.00% <ø> (ø)
taskiq/abc/schedule_source.py 100.00% <100.00%> (ø)
taskiq/abc/serializer.py 100.00% <100.00%> (ø)
taskiq/acks.py 100.00% <100.00%> (ø)
taskiq/api/__init__.py 100.00% <100.00%> (ø)
taskiq/cli/common_args.py 100.00% <100.00%> (ø)
taskiq/cli/watcher.py 0.00% <ø> (ø)
... and 37 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@s3rius
Copy link
Member

s3rius commented Oct 10, 2023

Since these mypy problems are totally minor, we can merge it now. I'll fix it later.

@s3rius s3rius merged commit cf87480 into taskiq-python:develop Oct 10, 2023
10 of 11 checks passed
@s3rius
Copy link
Member

s3rius commented Oct 10, 2023

Thank you very much for your contribution.

@s3rius s3rius mentioned this pull request Oct 10, 2023
@asvetlov
Copy link
Contributor Author

You are welcome! The next step is PR for taskiq-pipelines

@asvetlov asvetlov deleted the serializer branch October 11, 2023 06:59
@asvetlov
Copy link
Contributor Author

Could you please consider publishing a new release?

It is a pre-requirement for conversion taskiq-pipelines to use new serializers.

@s3rius
Copy link
Member

s3rius commented Oct 11, 2023

Sure.

@s3rius
Copy link
Member

s3rius commented Oct 11, 2023

Changes available in https://github.com/taskiq-python/taskiq/releases/tag/0.9.3.

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