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

♻️Rabbitmq rpc: response shall not be jsonized #4730

Merged
merged 8 commits into from
Sep 11, 2023

Conversation

sanderegg
Copy link
Member

@sanderegg sanderegg commented Sep 11, 2023

What do these changes do?

  • do not jsonize the RPC response.
  • have a RPCServerError

The concept is to have a common/shared interface module that defines the types of whatever is returned through RPC

Related issue/s

pre-requisite of #4703

How to test

DevOps Checklist

result = await func(*args, **kwargs)
return result
except asyncio.CancelledError:
_logger.debug("call was cancelled")
Copy link
Member

Choose a reason for hiding this comment

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

MINOR: some extra info?

Copy link
Member Author

Choose a reason for hiding this comment

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

for cancellation?

@codecov
Copy link

codecov bot commented Sep 11, 2023

Codecov Report

Merging #4730 (de200cc) into master (767354a) will increase coverage by 19.7%.
The diff coverage is 78.5%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #4730      +/-   ##
=========================================
+ Coverage    66.2%   85.9%   +19.7%     
=========================================
  Files         739     894     +155     
  Lines       33298   39110    +5812     
  Branches      198     543     +345     
=========================================
+ Hits        22059   33620   +11561     
+ Misses      11191    5366    -5825     
- Partials       48     124      +76     
Flag Coverage Δ
integrationtests 59.4% <ø> (-1.5%) ⬇️
unittests 84.2% <78.5%> (+5.4%) ⬆️

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

Files Changed Coverage Δ
...ice-library/src/servicelib/rabbitmq/_rpc_router.py 90.9% <75.0%> (ø)
...service-library/src/servicelib/rabbitmq/_errors.py 100.0% <100.0%> (ø)

... and 464 files with indirect coverage changes

@pcrespov
Copy link
Member

🎉 will start using this in my new PR! thx!

@sanderegg sanderegg added this to the the nameless milestone Sep 11, 2023
Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

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

👍

@codeclimate
Copy link

codeclimate bot commented Sep 11, 2023

Code Climate has analyzed commit de200cc and detected 0 issues on this pull request.

View more on Code Climate.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@sanderegg sanderegg merged commit e7018c7 into ITISFoundation:master Sep 11, 2023
@sanderegg sanderegg deleted the rabbitmq-rpc/no-json branch September 11, 2023 11:41
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Sep 22, 2023
50 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:clusters-keeper a:services-library issues on packages/service-libs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants