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

Separate thrift handler for matching service #3647

Conversation

vytautas-karpavicius
Copy link
Contributor

What changed?

  • Created Handler interface for matching service
  • Previous Handler struct renamed to HandlerImpl
  • Created ThriftHandler wrapper on top of Handler interface. At this point it only forward requests as is to underlying handler.
  • Created Mock for Handler interface.
  • Covered ThriftHandler with units tests. Test that requests are forwarded to underlying handler and response is returned as is.
    Replaced handler registration. Instead of registering HandlerImpl directly, wrap it with ThriftHandler and register it instead.

Why?
This is purely a mechanical refactoring that introduces additional ThriftHandler wrapper. For now it only forwards request to the original handler. Later on this will become a point for conversion between internal types and thrift types. Underlying handler will be independent of rpc protocol types.

How did you test it?

  • Covered ThriftHandler with unit tests
  • Started server locally

Potential risks
None, purely mechanical change.

@vytautas-karpavicius vytautas-karpavicius requested a review from a team October 14, 2020 13:34
@coveralls
Copy link

coveralls commented Oct 14, 2020

Coverage Status

Coverage increased (+0.05%) to 59.431% when pulling 2682b6d on vytautas-karpavicius:matching-thrift-handler into 242db1d on uber:master.

@vytautas-karpavicius vytautas-karpavicius merged commit ad06c81 into cadence-workflow:master Oct 26, 2020
github-actions bot pushed a commit to vytautas-karpavicius/cadence that referenced this pull request Feb 4, 2021
This is purely a mechanical refactoring that introduces additional ThriftHandler wrapper. For now it only forwards request to the original handler. Later on this will become a point for conversion between internal types and thrift types. Underlying handler will be independent of rpc protocol types.

-Created Handler interface for matching service
-Previous Handler struct renamed to HandlerImpl
-Created ThriftHandler wrapper on top of Handler interface. At this point it only forward requests as is to underlying handler.
-Created Mock for Handler interface.
-Covered ThriftHandler with units tests. Test that requests are forwarded to underlying handler and response is returned as is.
-Replaced handler registration. Instead of registering HandlerImpl directly, wrap it with ThriftHandler and register it instead.
yux0 pushed a commit to yux0/cadence that referenced this pull request May 4, 2021
This is purely a mechanical refactoring that introduces additional ThriftHandler wrapper. For now it only forwards request to the original handler. Later on this will become a point for conversion between internal types and thrift types. Underlying handler will be independent of rpc protocol types.

-Created Handler interface for matching service
-Previous Handler struct renamed to HandlerImpl
-Created ThriftHandler wrapper on top of Handler interface. At this point it only forward requests as is to underlying handler.
-Created Mock for Handler interface.
-Covered ThriftHandler with units tests. Test that requests are forwarded to underlying handler and response is returned as is.
-Replaced handler registration. Instead of registering HandlerImpl directly, wrap it with ThriftHandler and register it instead.
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