Updates README with working Thrift Java example, reference to custom Thrift compiler #105

Closed
wants to merge 7 commits into
from

Conversation

Projects
None yet
3 participants

No description provided.

How about:
"To create a Finagle Thrift service, you must use the custom Thrift compiler, which can be found here: thrift-0.5.0-finagle
This will generate, in addition to the regular thrift Iface interface, a ServiceIface interface that wraps all return values in a Future, which is required by Finagle."

whitespace change not needed

Owner

benpence replied Aug 26, 2012

Guess I didn't realize there were 2 spaces there. I'm getting old.

It's just generally good to avoid changing a line that doesn't need to be. This line is highlighted like there's a change but in fact there isn't.

Future

@stevegury stevegury and 2 others commented on an outdated diff Aug 27, 2012

@@ -182,6 +182,8 @@ Apache Thrift is a binary communication protocol that defines available methods
string hi();
}
+To create a Finagle Thrift service, you must use the <a href="https://github.com/mariusaeriksen/thrift-0.5.0-finagle">custom Thrift compiler</a>. It will generate, in addition to the regular Thrift `Iface` interface, a `ServiceIface` interface that wraps all return values in a `Future`, which is required by Finagle.
@stevegury

stevegury Aug 27, 2012

Contributor

We should recommend to use Scrooge instead https://github.com/twitter/scrooge
We will deprecate the custom thrift compiler in favor of Scrooge.

@isnotinvain

isnotinvain Aug 28, 2012

Contributor

Whichever is currently preferred should go in the docs I think. Do we currently use scrooge?

@benpence

benpence Aug 28, 2012

scrooge worked OK for me. I think it's important that the scrooge docs mention they need sbt v0.11.2 (or is that fixed by sbt-scrooge?). sbt v0.11.3 did not resolve when working against twitter dependencies.

@stevegury

stevegury Aug 28, 2012

Contributor

We use scrooge in birdcage but not yet in science (the migration should happen in a very near future).

And yes, you need to mention that sbt use a plugin that call scrooge to compile thrift files.
The latest plugin has only been release for sbt 0.11.2
(usually it's not a problem because we provide a sbt script that download the proper version,
see https://github.com/twitter/finagle/blob/master/sbt)

@benpence

benpence Aug 29, 2012

"To create a Finagle Thrift service, Finagle requiers you to generate, in addition to the regular Thrift Iface interface, a ServiceIface interface that wraps all return values in a Future. Use either scrooge or the sbt extension sbt-scrooge to compile your Thrift IDLs.

Note: scrooge and sbt-scrooge must build with the correct version of sbt."

Something like this? Or should the scrooge/sbt-scrooge install instructions reference the script instead?

@stevegury

stevegury Aug 29, 2012

Contributor

I would write something like that:
"""
To create a Finagle Thrift service, you just need to implement an Interface generated for Finagle by Scrooge. Scrooge is Thrift code generator similar to the standard Thrift compiler, the main difference is that the Interface expose asynchronous methods compatible with Finagle.

  • If you are using sbt to build your project, there is a plugin sbt-scrooge that will automatically compile IDL for you. Beware that Twitter has moved out from sbt, so the latest release version of this plugin is only compatible with sbt 0.11.2
  • If you are using maven, there is a maven pluggin "maven-finagle-thrift-plugin" that do the same thing.
    """

What do you think?

@isnotinvain

isnotinvain Aug 29, 2012

Contributor

I didn't know there is a maven plugin. Let's add that to the sample maven
file too.
On Aug 29, 2012 8:30 AM, "Steve Gury" notifications@github.com wrote:

In README.md:

@@ -182,6 +182,8 @@ Apache Thrift is a binary communication protocol that defines available methods
string hi();
}

+To create a Finagle Thrift service, you must use the custom Thrift compiler. It will generate, in addition to the regular Thrift Iface interface, a ServiceIface interface that wraps all return values in a Future, which is required by Finagle.

I would write something like that:
"""
To create a Finagle Thrift service, you just need to implement an
Interface generated for Finagle by Scroogehttps://github.com/twitter/scrooge.
Scrooge https://github.com/twitter/scrooge is Thrift code generator
similar to the standard Thrift compiler http://thrift.apache.org, the
main difference is that the Interface expose asynchronous methods
compatible with Finagle.

  • If you are using sbt to build your project, there is a plugin
    sbt-scrooge https://github.com/twitter/sbt-scrooge that will
    automatically compile IDL for you. Beware that Twitter has moved out from
    sbt, so the latest release version of this plugin is only compatible with
    sbt 0.11.2
  • If you are using maven, there is a maven pluggin
    "maven-finagle-thrift-plugin" that do the same thing. """

What do you think?


Reply to this email directly or view it on GitHubhttps://github.com/twitter/finagle/pull/105/files#r1486589.

@benpence

benpence Aug 30, 2012

"""
To create a Finagle Thrift service, you must implement the ServiceIface Interface that Scrooge (a custom Thrift compiler) generates for your service. Scrooge wraps your service method return values with asynchronous Future objects to be compatible with Finagle.

  • If you are using sbt to build your project, the sbt-scrooge plugin automatically compiles your Thrift IDL. Note: The latest release version of this plugin is only compatible with sbt 0.11.2
  • If you are using maven to manage your project, maven-finagle-thrift-plugin can also compile Thrift IDL for Finagle.
    """

@stevegury Steve, do you have an official link for the maven plugin? I could only find it under Twitter's maven repos. Sorry, I'm not familiar with maven plugins. Let me know if the revision is OK. I think it's important to specify that the developer must implement ServiceIface and not Iface to make the example clearer.
@isnotinvain that is a good idea.

@stevegury

stevegury Aug 30, 2012

Contributor

The source of the maven-finagle-thrift-plugin is in the internal repo "birdcage" in the maven-plugins directory, we currently use the version 0.0.5
It seems that it is not yet open sourced, you should talk to tools team about that.

@isnotinvain

isnotinvain Aug 30, 2012

Contributor

+1 / Ship It! for @benpence's latest revision. It is clear and accurate regardless of whether maven-finagle-thrift-plugin is open sourced yet (the binaries are available right?)

@stevegury We can work on open sourcing maven-finagle-thrift-plugin separately.

Contributor

isnotinvain commented Sep 6, 2012

Bump!
I say ship it! What's the next step?

Contributor

stevegury commented Sep 6, 2012

I was waiting for a change on the line 185, like @benpence mentioned.
After that, I will pull this commit into the internal repo, and it will appear a little bit after on the github repo.

Contributor

stevegury commented Sep 10, 2012

I pulled this internally.
You should show it soon in the github repo

stevegury closed this Sep 10, 2012

@kil9 kil9 pushed a commit to kil9/finagle that referenced this pull request Feb 26, 2014

@stevegury Ben Pence + stevegury [split] Updates README with working Thrift Java example, reference to…
… custom Thrift compiler

Github-pull-request: twitter#105
Signed-off-by: Steve Gury <stevegury@twitter.com>

RB_ID=85423
a8da1e7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment