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

Generate traits decoupled from Thrift dependencies #324

Open
yawaramin opened this issue Feb 26, 2020 · 3 comments
Open

Generate traits decoupled from Thrift dependencies #324

yawaramin opened this issue Feb 26, 2020 · 3 comments

Comments

@yawaramin
Copy link

yawaramin commented Feb 26, 2020

Currently, the generated code is strongly coupled to Thrift. This locks the whole codebase into Thrift, or forces developers to manually duplicate types and convert between them. Instead, the generated code could decouple from Thrift. Here's a simplified example.

struct Address {
  1: required string address1
  2: optional string address2
}

Currently this generates:

object Address {
  ...
}

trait Address
  extends ThriftStruct
  with ... {
  ...
  def address1: String
  def address2: Option[String]
  ...
}

I am suggesting generating completely decoupled trait:

trait IAddress {
  def address1: String
  def address2: Option[String]
}

object Address...

trait Address
  extends ThriftStruct
  with IAddress // <= NOTE: the existing trait now extends the new decoupled trait
  with ... {
  ...
  def address1: String
  def address2: Option[String]
  ...
}

Now because of this subtyping relationship, any codebase that wants to stay decoupled from Thrift–say, domain model or business logic code–can use the decoupled IAddress trait instead of the strongly-coupled Address trait.

Note that the naming is a suggestion only, it's not the important part of this proposal; the important part is the decoupling from Thrift.

@ryanoneill
Copy link
Contributor

Hi @yawaramin, thanks for the suggestion. We're going to be making some other big changes to Scrooge this year in order to better align it with Finagle and bring Java code generation in line with Scala's. Perhaps this is something that we would consider after that.

Internally, we've been trying to cut down on the amount of code scrooge generates, as the time it takes to compile all of it can make it one of the largest contributors to slow compile times throughout the company. So this is something that we'd need to evaluate from multiple angles.

@tkolleh
Copy link

tkolleh commented Aug 10, 2022

@ryanoneill what are some strategies used at Twitter to reduce compile times. I'm facing something similar. Compiling in general is coffee + restroom break long at best and it makes using Metals nerve-racking.

@ryanoneill
Copy link
Contributor

@tkolleh hey, sorry, just saw your response to me here. In 2020, I ended up leaving the team responsible for Scrooge and then ended up leaving Twitter entirely earlier this year. So, I can't give insight on the team or company's current plans for Scrooge.

Compile time reduction has mostly been focused on by using finer grained dependencies via pants and bazel. That combined with a buildcache makes it bearable.

I haven't looked at the current state of Scrooge in about two years now, but I would be trying to reduce the amount of code that Scrooge generates for you via flags. The more code that gets generated, the more code that needs to be compiled. Almost no one is using everything that gets generated either. So it's a waste from just not knowing when it's generating how it's going to be used. If you do know, there are things that likely can be done to cut it down. Those likely would require pull requests to implement though. Best of luck with things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants