Thrift
  1. Thrift
  2. THRIFT-538

missing support for framed transport

    Details

    • Type: Wish Wish
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
      None

      Description

      It is somewhat frustrating when writing an application to have to choose a transport that will ultimately create compatibility issues for one or more otherwise supported languages. This is currently the case because some clients require a framed transport (i.e. Twisted), while others lack an implementation of one (i.e. Haskell, Cocoa, etc).

      If every language supported the framed transport, it would even be possible to make framed the only option.

      1. Framed.hs
        4 kB
        Martin Grabmueller
      2. Framed-Int32.diff
        1 kB
        Kirk Peterson

        Issue Links

          Activity

          Hide
          Stanislav Yudin added a comment -

          Is there any progress on that issue? Missing framed support in cocoa really disappointing.

          Show
          Stanislav Yudin added a comment - Is there any progress on that issue? Missing framed support in cocoa really disappointing.
          Hide
          Bryan Duxbury added a comment -

          I would eagerly commit framed transport implementations for these languages if someone provided them. They should be trivial.

          Show
          Bryan Duxbury added a comment - I would eagerly commit framed transport implementations for these languages if someone provided them. They should be trivial.
          Hide
          Martin Grabmueller added a comment -

          This is a first attempt at adding framed transport for Haskell. Unfortunately, my thrift build setup is so botched at the moment that I couldn't test it properly. I would appreciate if anyone could test it against a framed server.

          Show
          Martin Grabmueller added a comment - This is a first attempt at adding framed transport for Haskell. Unfortunately, my thrift build setup is so botched at the moment that I couldn't test it properly. I would appreciate if anyone could test it against a framed server.
          Hide
          Christian Lavoie added a comment -

          Committed Haskell client support with rev 1068756, after running a few tests against a dumb cpp server.

          Thanks for the contribution Martin, much appreciated.

          Show
          Christian Lavoie added a comment - Committed Haskell client support with rev 1068756, after running a few tests against a dumb cpp server. Thanks for the contribution Martin, much appreciated.
          Hide
          Kirk Peterson added a comment - - edited

          The length function of the Data.ByteString.Lazy package used on line 74 returns an 8 byte integer (Int64) for the frame length, where in the Java TFramedTransport class (see: line 156) is using a 4 byte integer to encode the frame length: transport_.write(i32buf, 0, 4).

          We see this again on line 84 we're using Data.byteString.Lazy's decode to convert the 4 bytes read in on line 83 into an Int representation of the size of the frame to be used as the length parameter of tRead on line 87, haskell Int's are 8 bytes in length, where in TFramedTransport line 129 we see another case of expecting a 4 byte encoding of the frame length: transport_.readAll(i32buf, 0, 4);

          Cassandra 0.7 has switched to using TFramedTransport in it's Thrift daemon, which this module (as of revision 1072638) will not work with.
          I've attached a diff that changes lines 74 and 84 to use Data.Int's Int32, which I have working when tested against cassandra 0.7.2.

          Show
          Kirk Peterson added a comment - - edited The length function of the Data.ByteString.Lazy package used on line 74 returns an 8 byte integer (Int64) for the frame length, where in the Java TFramedTransport class (see: line 156) is using a 4 byte integer to encode the frame length: transport_.write(i32buf, 0, 4). We see this again on line 84 we're using Data.byteString.Lazy's decode to convert the 4 bytes read in on line 83 into an Int representation of the size of the frame to be used as the length parameter of tRead on line 87, haskell Int's are 8 bytes in length, where in TFramedTransport line 129 we see another case of expecting a 4 byte encoding of the frame length: transport_.readAll(i32buf, 0, 4); Cassandra 0.7 has switched to using TFramedTransport in it's Thrift daemon, which this module (as of revision 1072638) will not work with. I've attached a diff that changes lines 74 and 84 to use Data.Int's Int32, which I have working when tested against cassandra 0.7.2.
          Hide
          Christian Lavoie added a comment -

          Nice catch Kirk, thanks!

          Committed revision 1072684.

          Show
          Christian Lavoie added a comment - Nice catch Kirk, thanks! Committed revision 1072684.
          Hide
          Geert-Johan Riemer added a comment -
          Show
          Geert-Johan Riemer added a comment - There is a patch for cocoa. https://issues.apache.org/jira/browse/THRIFT-1503
          Hide
          John Chee added a comment -

          This ticket is closable. The components referenced in this ticket have Framed Transports:

          Haskell - Library: https://github.com/apache/thrift/blob/master/lib/hs/src/Thrift/Transport/Framed.hs
          Cocoa - Library: https://github.com/apache/thrift/blob/master/lib/cocoa/src/transport/TFramedTransport.m

          cc: Roger Meier

          Show
          John Chee added a comment - This ticket is closable. The components referenced in this ticket have Framed Transports: Haskell - Library: https://github.com/apache/thrift/blob/master/lib/hs/src/Thrift/Transport/Framed.hs Cocoa - Library: https://github.com/apache/thrift/blob/master/lib/cocoa/src/transport/TFramedTransport.m cc: Roger Meier
          Hide
          Roger Meier added a comment -

          implemented by other tickets as mentioned by John Chee.

          Show
          Roger Meier added a comment - implemented by other tickets as mentioned by John Chee.

            People

            • Assignee:
              Unassigned
              Reporter:
              Eric Evans
            • Votes:
              2 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development