Skip to content

Commit

Permalink
Add ThriftTransport constructor taking plain EventBase
Browse files Browse the repository at this point in the history
Summary:
Currently, the generated `ThriftTransport` constructor accepts a `VirtualEventBase&`. This is for consistency
with `AsyncMcClient` code. However, currently `ThriftTransport` doesn't use the virtual-ness of `VirtualEventBase` at all
and simply directly uses the backing `EventBase`. For tests, it would be convenient to be able to construct a `ThriftTransport`
directly from `EventBase`, which is what this diff adds.

Reviewed By: stuclar

Differential Revision: D17056270

fbshipit-source-id: f968417e95952f48c58c087bf0d3d4035ee46267
  • Loading branch information
jmswen authored and facebook-github-bot committed Aug 28, 2019
1 parent a83eea8 commit eb7e1ed
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ namespace memcache {
template <>
class ThriftTransport<hellogoodbye::HelloGoodbyeRouterInfo> : public ThriftTransportBase {
public:
ThriftTransport(folly::VirtualEventBase& eventBase, ConnectionOptions options)
ThriftTransport(folly::EventBase& eventBase, ConnectionOptions options)
: ThriftTransportBase(eventBase, std::move(options)) {}
ThriftTransport(folly::VirtualEventBase& eventBase, ConnectionOptions options)
: ThriftTransportBase(eventBase.getEventBase(), std::move(options)) {}
~ThriftTransport() override final = default;

void setFlushList(FlushList* flushList) override final {
Expand Down
6 changes: 2 additions & 4 deletions mcrouter/lib/network/ThriftTransport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

#include <folly/fibers/FiberManager.h>
#include <folly/io/async/EventBase.h>
#include <folly/io/async/VirtualEventBase.h>
#include <thrift/lib/cpp/async/TAsyncSocket.h>
#include <thrift/lib/cpp2/async/RequestChannel.h>

Expand All @@ -24,10 +23,9 @@ namespace facebook {
namespace memcache {

ThriftTransportBase::ThriftTransportBase(
folly::VirtualEventBase& eventBase,
folly::EventBase& eventBase,
ConnectionOptions options)
: eventBase_(eventBase.getEventBase()),
connectionOptions_(std::move(options)) {}
: eventBase_(eventBase), connectionOptions_(std::move(options)) {}

void ThriftTransportBase::closeNow() {
resetClient();
Expand Down
6 changes: 2 additions & 4 deletions mcrouter/lib/network/ThriftTransport.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,7 @@ class ThriftTransportBase : public Transport,
private folly::AsyncSocket::ConnectCallback,
private apache::thrift::CloseCallback {
public:
ThriftTransportBase(
folly::VirtualEventBase& eventBase,
ConnectionOptions options);
ThriftTransportBase(folly::EventBase& eventBase, ConnectionOptions options);
virtual ~ThriftTransportBase() override = default;

static constexpr folly::StringPiece name() {
Expand Down Expand Up @@ -127,7 +125,7 @@ template <class RouterInfo>
class ThriftTransport : public ThriftTransportBase {
public:
ThriftTransport(folly::VirtualEventBase& eventBase, ConnectionOptions options)
: ThriftTransportBase(eventBase, std::move(options)) {}
: ThriftTransportBase(eventBase.getEventBase(), std::move(options)) {}
~ThriftTransport() override final = default;

template <class Request>
Expand Down
4 changes: 3 additions & 1 deletion mcrouter/lib/network/gen/MemcacheThriftTransport.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ namespace memcache {
template <>
class ThriftTransport<MemcacheRouterInfo> : public ThriftTransportBase {
public:
ThriftTransport(folly::VirtualEventBase& eventBase, ConnectionOptions options)
ThriftTransport(folly::EventBase& eventBase, ConnectionOptions options)
: ThriftTransportBase(eventBase, std::move(options)) {}
ThriftTransport(folly::VirtualEventBase& eventBase, ConnectionOptions options)
: ThriftTransportBase(eventBase.getEventBase(), std::move(options)) {}
~ThriftTransport() override final = default;

void setFlushList(FlushList* flushList) override final {
Expand Down

0 comments on commit eb7e1ed

Please sign in to comment.