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

examples/aio_pika_server.py: Fix example add_user() to not trigger mypy #90

Merged
merged 2 commits into from
Nov 14, 2022

Conversation

bernhardkaindl
Copy link
Contributor

@bernhardkaindl bernhardkaindl commented Sep 21, 2022

Hello!

This is a minor improvement to fix mypy of a currently not used RPC endpoint function in examples/aio_pika_server.py:

Details:

The add_user() example in examples/aio_pika_server.py
is not yet used by the example client, so it can be fixed for mypy
using dataclasses similar to the example add_user()
with dataclasses at the end of README.rst:
https://github.com/dapper91/pjrpc/blob/master/README.rst#open-api-specification

-def add_user(message: aio_pika.IncomingMessage, user: dict):
-    user_id = uuid.uuid4().hex
-
-    return {'id': user_id, **user}
+def add_user(message: aio_pika.IncomingMessage, user_info: UserInfo) -> AddedUser:
+    """Simluate the creation of a user: Receive user info and return it with an uuid4.
+    :param UserInfo user_info: user data
+    :returns: user_info with a randomly generated uuid4 added
+    :rtype: AddedUser"""
+    return AddedUser(**user_info.__dict__, uuid=uuid.uuid4())

@dapper91: The 1st commit of this branch is the commit from #89 as it's context depends on it.
It can be merged after (or instead of) #89.

It reduces the coverage percentage very slightly because the updated lines in the unused function is not yet covered. If this is approved or merged, a test case covering using RabbitMQ in the github actions:

Bernhard Kaindl added 2 commits September 21, 2022 19:49
The add_user() example in examples/aio_pika_server.py is not yet used
by the example client, so it can be fixed for mypy using dataclasses
similar to the example add_user() with dataclasses at the end of README.rst.
@codecov-commenter
Copy link

codecov-commenter commented Sep 21, 2022

Codecov Report

Merging #90 (5b168b4) into dev (064c5c6) will increase coverage by 0.05%.
The diff coverage is 97.63%.

@@            Coverage Diff             @@
##              dev      #90      +/-   ##
==========================================
+ Coverage   79.49%   79.55%   +0.05%     
==========================================
  Files          41       41              
  Lines        2717     2719       +2     
==========================================
+ Hits         2160     2163       +3     
+ Misses        557      556       -1     
Flag Coverage Δ
unittests 79.55% <97.63%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pjrpc/client/backend/aio_pika.py 0.00% <0.00%> (ø)
pjrpc/client/backend/kombu.py 0.00% <0.00%> (ø)
pjrpc/__about__.py 100.00% <100.00%> (ø)
pjrpc/client/client.py 97.55% <100.00%> (ø)
pjrpc/client/integrations/pytest.py 91.85% <100.00%> (ø)
pjrpc/common/__init__.py 91.66% <100.00%> (ø)
pjrpc/common/common.py 89.28% <100.00%> (+0.82%) ⬆️
pjrpc/common/exceptions.py 98.64% <100.00%> (ø)
pjrpc/common/v20.py 91.22% <100.00%> (ø)
pjrpc/server/dispatcher.py 87.73% <100.00%> (+0.04%) ⬆️
... and 5 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor Author

@bernhardkaindl bernhardkaindl left a comment

Choose a reason for hiding this comment

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

Looks good, @dapper91 - may I merge?

Thanks, Bernhard

@bernhardkaindl bernhardkaindl merged commit 1524ecd into dev Nov 14, 2022
@dapper91 dapper91 deleted the mypy-example-aio_pika-fix-add_user branch August 10, 2023 15:12
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.

None yet

2 participants