Skip to content

Commit

Permalink
scrooge/finagle: Remove generated HKT, FutureIface and their friends
Browse files Browse the repository at this point in the history
Problem

FutureIface is deprecated.
Scrooge is more powerful when used with finagle integration,  and in that case, higher-kinded-types won't play its advantages.
Scrooge generated code is growing which increases source compilation time, we should get rid of the unused parts.

Solution

Remove them from code-gen.
In generated code, replace HKT/FutureIface with MethodPerEndpoint.
Remove some deprecated builders in both scrooge-gen and finagle that require HKT as the parameter.

Differential Revision: https://phabricator.twitter.biz/D747744
  • Loading branch information
yufangong authored and jenkins committed Sep 30, 2021
1 parent 598f73e commit fc21ccc
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 63 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ Note that ``PHAB_ID=#`` and ``RB_ID=#`` correspond to associated messages in com
Unreleased
----------

Breaking API Changes
~~~~~~~~~~~~~~~~~~~~

* finagle-thrift: Removed c.t.finagle.thrift.ThriftClient#newMethodIface and
ThriftClient#thriftService, use c.t.f.thrift.ThriftClient#methodPerEndpoint. ``PHAB_ID=D747744``

Bug Fixes
~~~~~~~~~~

Expand Down
Original file line number Diff line number Diff line change
@@ -1,27 +1,13 @@
package com.twitter.finagle.thrift

import com.twitter.finagle.thrift.service.{MethodPerEndpointBuilder, ThriftServiceBuilder}
import com.twitter.finagle.thrift.service.MethodPerEndpointBuilder

/**
* Stateless helper methods which wrap a given `ServiceIface` (deprecated) or a
* given `ServicePerEndpoint` with another type via the given method's implicit Builder.
*/
trait ThriftClient {

/**
* Converts from a Service interface (`ServiceIface`) to the
* method interface (`newIface`).
*/
@deprecated(
"Use com.twitter.finagle.ThriftClient#methodPerEndpoint[ServicePerEndpoint, MethodPerEndpoint]",
"2017-11-13"
)
def newMethodIface[ServiceIface, FutureIface](
serviceIface: ServiceIface
)(
implicit builder: MethodIfaceBuilder[ServiceIface, FutureIface]
): FutureIface = builder.newMethodIface(serviceIface)

/**
* Converts from a Service interface (`ServicePerEndpoint`) to the
* method interface (`MethodPerEndpoint`).
Expand All @@ -31,15 +17,4 @@ trait ThriftClient {
)(
implicit builder: MethodPerEndpointBuilder[ServicePerEndpoint, MethodPerEndpoint]
): MethodPerEndpoint = builder.methodPerEndpoint(servicePerEndpoint)

/**
* Converts from a Service interface (`ServicePerEndpoint`) to the higher-kinded
* method interface (`MethodPerEndpoint`).
*/
@deprecated("Use methodPerEndpoint", "2018-01-12")
def thriftService[ServicePerEndpoint, ThriftServiceType](
servicePerEndpoint: ServicePerEndpoint
)(
implicit builder: ThriftServiceBuilder[ServicePerEndpoint, ThriftServiceType]
): ThriftServiceType = builder.build(servicePerEndpoint)
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package com.twitter.finagle.thrift

import com.twitter.finagle.stats.{NullStatsReceiver, StatsReceiver}
import com.twitter.finagle.{Service, Thrift}
import com.twitter.finagle.stats.NullStatsReceiver
import com.twitter.finagle.stats.StatsReceiver
import com.twitter.finagle.Service
import com.twitter.finagle.Thrift
import java.lang.reflect.Constructor
import org.apache.thrift.protocol.TProtocolFactory

Expand Down Expand Up @@ -142,7 +144,7 @@ private[twitter] object ThriftUtil {
val baseName: String = stripSuffix(iface)
(for {
serviceCls <- findClass[BinaryService](baseName + FinagledServerSuffixScala)
baseClass <- findClass1(baseName)
baseClass <- findClass1(baseName + "$MethodPerEndpoint")
} yield {
findConstructor(
serviceCls,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ package com.twitter.finagle.thrift

import com.twitter.finagle._
import com.twitter.finagle.service.ResponseClassifier
import com.twitter.finagle.stats.{NullStatsReceiver, StatsReceiver}
import com.twitter.finagle.stats.NullStatsReceiver
import com.twitter.finagle.stats.StatsReceiver
import com.twitter.finagle.thrift.service.Filterable
import org.apache.thrift.protocol.TProtocolFactory

Expand Down Expand Up @@ -57,19 +58,3 @@ trait ServiceIfaceBuilder[ServiceIface <: Filterable[ServiceIface]] {
newServiceIface(thriftService, clientParam)
}
}

/**
* A typeclass to construct a MethodIface by wrapping a ServiceIface.
* This is a compatibility constructor to replace an existing Future interface
* with one built from a ServiceIface.
*
* Scrooge generates implementations of this builder.
*/
@deprecated("Use com.twitter.finagle.thrift.service.MethodPerEndpointBuilder", "2017-11-13")
trait MethodIfaceBuilder[ServiceIface, MethodIface] {

/**
* Build a FutureIface wrapping a ServiceIface.
*/
def newMethodIface(serviceIface: ServiceIface): MethodIface
}

This file was deleted.

0 comments on commit fc21ccc

Please sign in to comment.