Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-538

missing support for framed transport

Details

    • Wish
    • Status: Closed
    • Major
    • Resolution: Fixed
    • None
    • None
    • 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.

      Attachments

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

        Issue Links

          Activity

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

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

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

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

            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.

            mgrabmueller 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.

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

            Thanks for the contribution Martin, much appreciated.

            clavoie 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.
            necrobious 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.

            necrobious 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.

            Nice catch Kirk, thanks!

            Committed revision 1072684.

            clavoie Christian Lavoie added a comment - Nice catch Kirk, thanks! Committed revision 1072684.
            geertjohan Geert-Johan Riemer added a comment - There is a patch for cocoa. https://issues.apache.org/jira/browse/THRIFT-1503
            cheecheeo 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

            cheecheeo 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
            roger Roger Meier added a comment -

            implemented by other tickets as mentioned by John Chee.

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

            People

              Unassigned Unassigned
              urandom Eric Evans
              Votes:
              2 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: