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

THRIFT-3299 Dart language bindings in Thrift #608

Closed
wants to merge 69 commits into from

Conversation

stevenosborne-wf
Copy link

Description: Add bindings for dart.

Ticket: https://issues.apache.org/jira/browse/THRIFT-3299

markerickson-wf and others added 30 commits August 29, 2015 10:46
https://issues.apache.org/jira/browse/THRIFT-3299

Cleanup some issues in the generator found while analyzing generated code.
https://issues.apache.org/jira/browse/THRIFT-3299

Switch service generation of service server code to opt-in instead of opt-out.
https://issues.apache.org/jira/browse/THRIFT-3299

Optimizations when using List.length since the base implementation iterates over elements in the getter.
https://issues.apache.org/jira/browse/THRIFT-3299

Fix generation problems exposed by tutorial specification, related to inheritance and constants.
https://issues.apache.org/jira/browse/THRIFT-3299

In progress.  Initial stab at tutorial implementation, with known issues.  Changed socket interface for open / close to be async.  Added support for sockets that are listening for messages (server).
https://issues.apache.org/jira/browse/THRIFT-3299

Fix bugs in binary protocol and json protocol.  Use base 64 encoding for socket and http transports.  Implement the remainder of the tutorial.
https://issues.apache.org/jira/browse/THRIFT-3299

JSON protocol unit tests.  Changed binary type to Uint8List.  Cleaned up tutorial shell script.
https://issues.apache.org/jira/browse/THRIFT-3299

Move completer logic from WebSocket implementations to transport.  Split TSocketTransport into TClientSocketTransport and TServerSocketTransport to cleanup some conditional logic.
https://issues.apache.org/jira/browse/THRIFT-3299

Move Base 64 encoding out of transport and back into web_socket implementations.
https://issues.apache.org/jira/browse/THRIFT-3299

Return Futures from open and close that actually reflect the state of the WebSocket.
Cleanup TSocketTransport and related code
https://issues.apache.org/jira/browse/THRIFT-3299

Avoid using String.codeUnits as a Uint8List.  Cleanup bad type assignments in tutorial client.
@asfbot
Copy link

asfbot commented Sep 21, 2015

"Jens Geyer" on mark.erickson@workiva.com replies:
Hi Mark,

(1) The current status quo is that the 0.9.3 release will still rely on =
autotools as the primary build system, while the next release very =
likely will be based on CMake. There is already some work done, but I =
personally don=E2=80=99t have the full overview about the current status =
of the CMake support, so you may ask for the details on the mailing list =
if such becomes necessary. I=E2=80=99ll cross-post this to the dev-list.

(2) Moving entries to /.gitignore would be good.=20
packages, for instance

Should be possible by means of entries that exclude only files in a =
given subfolder, similar to=20

/lib/csharp/**/bin/
/lib/delphi/test/*.dcu

(3) The ThriftTest integration test is a cornerstone of Thrift. If the =
Dart code is compatible with Dart, well that=E2=80=99s a good start. ;-) =
but not really sufficient. As always, the devil=E2=80=99s in the =
details. The ThriftTest is designed to validate cross-language and =
cross-platform compatibility in a standardized manner. And it gives us =
reviewers something to play with, besides the (comparingly simple) =
tutorial stuff.=20

Some form of a self-test is usually included when =E2=80=9Cmake =
check=E2=80=9D is run, typically a server/client pair of that language =
(given you have a server, of course) using the ThriftTest integration =
test code. A question that frequently raises is the location of the =
tests. This has evolved over time, and the current consensus and =
rule-of-thumb is this:

(a) the ThriftTest integration test should be placed under =

/test/
(b) any other, especially language-specific tests, should be placed =
under /lib//test=20
yet. [...] using Dart for a server is far less common.

I think that=E2=80=99s fine.=20

(4) suggestions

Would it be possible to connect the tutorial to =E2=80=9Cmake =
check=E2=80=9D as well? That would be awesome.

Have fun,
JensG

From: Mark Erickson=20
Sent: Monday, September 21, 2015 1:08 AM
To: Jens Geyer=20
Cc: Steven Osborne=20
Subject: Re: Fw: [GitHub] thrift pull request: THRIFT-3299 Dart language =
bindings in Thrift

Hi Jens,=20

  1. I did not intentionally introduce a dependency on CMake. My changes =
    to build files resulted from looking for usages of as3, and adding =
    Dart support where it seemed appropriate. I may have missed something =
    here?

  2. I can move the .gitignore entries for Dart to the main file. I added =
    a local file to avoid unintentional side-effects with other languages, =
    by ignoring packages, for instance. But it looks like it should be =
    fine to move it to the root file.

  3. That's correct, we have not implemented the ThriftTest integration =
    test yet. We can work on this - it looks like the php implementation is =
    pretty straightforward, so I might start from that. Like the php test, =
    the Dart would only test the client, since we don't plan to implement a =
    full Dart Thrift server implementation yet. Of course someone else =
    could add this later, but using Dart for a server is far less common.

Thank you for looking at the Dart PR. Please let us know if you have =
any other questions or suggestions. Thanks,

Mark

Mark Erickson
Senior Software Engineer
Workiva Inc.

On Sat, Sep 19, 2015 at 7:36 AM, Jens Geyer jensgeyer@hotmail.com =wrote:

Hi Mark,

a few comments, just trying to get it running on my machine.

  1. Looks as if I have to use CMake? Just asking ... no need to panic. =
    :-)
  2. We usually don't use local .gitgnore files, only one in the root =
    folder
  3. I found the tutorial, but the integration test (ThriftTest.thrift) =
    is probably still work in progress?

Thanks + keep up the good work,
JensG

-----Urspr=C3=BCngliche Nachricht----- From: stevenosborne-wf
Sent: Thursday, September 17, 2015 9:24 PM
To: dev@thrift.apache.org
Subject: [GitHub] thrift pull request: THRIFT-3299 Dart language =
bindings in Thrift

GitHub user stevenosborne-wf opened a pull request:

 https://github.com/apache/thrift/pull/608

 THRIFT-3299 Dart language bindings in Thrift

 Description: Add bindings for [dart](https://www.dartlang.org/).

 Ticket: https://issues.apache.org/jira/browse/THRIFT-3299

You can merge this pull request into a Git repository by running:

 $ git pull https://github.com/markerickson-wf/thrift thrift-dart

Alternatively you can review and apply these changes as the patch at:

 https://github.com/apache/thrift/pull/608.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

 This closes #608

commit 97dcdf2
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-08-29T15:46:51Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Added t_dart_generator.cc and add as a target for Make.

commit 11473fd
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-08-30T01:08:18Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Cleanup some issues in the generator found while analyzing =

generated code.

commit 221f604
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-08-30T01:23:28Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Initial port of library code, except for transport implementations.

commit bc73bae
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-04T04:05:30Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Minor tweaks to the dart generator

commit fc049c0
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-04T04:09:24Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Added binary and JSON protocols.  Added WebSocket transport.

commit 495c0dd
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-04T18:18:03Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Switch service generation of service server code to opt-in instead =

of opt-out.

commit ecce971
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-04T18:37:16Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Added THttpTransport and a did little refactoring.

commit a01e06a
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-04T20:28:42Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Added BufferedTransport and FramedTransport, with some refactoring.

commit 84e8763
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-04T21:14:56Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Added MultiplexedProtocol.

commit 08d0ce2
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-04T21:25:08Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Ran dartfmt.

commit c410c6d
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-05T03:11:31Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Optimizations when using List.length since the base implementation =

iterates over elements in the getter.

commit 6195d22
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-06T16:57:46Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Fix BufferedTransport.flush to be able to read from the write =

buffer.

commit e186fb6
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-08T15:05:56Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Fix typo in Dart SDK requirement

commit 017cbd7
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-08T15:52:56Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Add pubspec.yaml to generated files.

commit e347643
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-08T20:07:43Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Fix generation problems exposed by tutorial specification, related =

to inheritance and constants.

commit 21b97b2
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-08T21:59:18Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Initialize seqid and increment when client writes message begin.

commit dbf2878
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-10T16:11:19Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 In progress.  Initial stab at tutorial implementation, with known =

issues. Changed socket interface for open / close to be async. Added =
support for sockets that are listening for messages (server).

commit db111ae
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-11T03:10:13Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Fix bugs in binary protocol and json protocol.  Use base 64 =

encoding for socket and http transports. Implement the remainder of the =
tutorial.

commit be267af
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-11T03:14:22Z

 Merge branch 'master' into thrift-dart

commit c18f92c
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-11T18:20:10Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Make pubspec dependencies more consistent and restrictive.

commit 3e76361
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-11T20:12:08Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Switch uses of List<int> to Uint8List to be more explicit.

commit 4c74741
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-11T20:50:41Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Complete with error when a socket is closed.  Minor code cleanup.

commit 1cbb3eb
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-11T23:22:55Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Writing unit tests.  A little code cleanup.

commit d97bcc1
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-12T02:23:30Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 JSON protocol unit tests.  Changed binary type to Uint8List.  =

Cleaned up tutorial shell script.

commit e236093
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-12T04:00:00Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Protocol and transport unit tests.

commit 8d74526
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-14T15:23:09Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Move completer logic from WebSocket implementations to transport.  =

Split TSocketTransport into TClientSocketTransport and =
TServerSocketTransport to cleanup some conditional logic.

commit 4a5197e
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-14T16:17:21Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Move Base 64 encoding out of transport and back into web_socket =

implementations.

commit cd83635
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-14T16:35:52Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Return Futures from open and close that actually reflect the state =

of the WebSocket.

commit 7bacb04
Author: Mark Erickson mailto:mark.erickson%2Bgithub@workiva.com
Date: 2015-09-14T17:34:19Z

 Merge pull request #2 from markerickson-wf/socket-transport

 Cleanup TSocketTransport and related code

commit 544834b
Author: Mark Erickson mark.erickson@workiva.com
Date: 2015-09-14T20:48:01Z

 Create an Apache Thrift language binding for Dart (dartlang.org).
 https://issues.apache.org/jira/browse/THRIFT-3299

 Avoid using String.codeUnits as a Uint8List.  Cleanup bad type =

assignments in tutorial client.



If your project is set up for it, you can reply to this email and have =
your
reply appear on GitHub as well. If your project does not have this =
feature
enabled and wishes so, or if the feature is enabled but not working, =
please
contact infrastructure at infrastructure@apache.org or file a JIRA =
ticket
with INFRA.
---=20

https://issues.apache.org/jira/browse/THRIFT-3299

Use a sync completer to ensure that the buffer can be read immediately after the read buffer is set, and avoid a race condition where another response could overwrite the read buffer.
https://issues.apache.org/jira/browse/THRIFT-3299

Change to BinaryProtocol to make tutorial clients more interchangeable.  Add support in the tutorial server for TCP socket connections, in addition to WebSockets.
@asfbot
Copy link

asfbot commented Sep 21, 2015

Mark Erickson on jensgeyer@hotmail.com replies:
Hi Jens,

  1. I have a branch where I added Dart make support by modifying
    configure.ac and lib/Makefile.am, test/Makefile.am, and
    tutorial/Makefile.am, and adding the Dart Makefile.am files. It looks like
    it's working (it will properly include / exclude Dart, depending on
    presence of Dart SDK binaries), so I'll get that in after it's code
    reviewed. That will work with the current autotools, but it sounds like
    there is more to do for CMake.

  2. Thanks, I'll just add the Dart specific bits to the appropriate
    subdirectories, as you suggested.

  3. I'm working on the integration test now, so I should have something done
    soon.

  4. I'm not sure yet what you mean by connecting the tutorial to make check. If you have an example you could point me to, or could expand on
    that, it would be appreciated.

Thanks,

Mark

Mark Erickson
Senior Software Engineer
Workiva Inc.

2900 University Blvd, Ames, IA 50010
Email: mark.erickson@workiva.com
Mobile: 515.257.6865
e support,
but not
iftTest is
=80=9D is run,
e,
g>
=9D as well? That
o
a
o
t
y
:
)
er
s
.
re
t

markerickson-wf and others added 13 commits September 21, 2015 18:18
Add a Dart console-client to the tutorial
Coerce null primitives to default values on write
https://issues.apache.org/jira/browse/THRIFT-3299

Added Dart support in build files.  Integration tests working with Python Server on buffered transport.  Fixed bug in generator with iterating over maps.  Fixed typo in Python test server.  Discovered bug in FramedTransport, which is not fixed yet.
https://issues.apache.org/jira/browse/THRIFT-3299

Make TClientSocketTransport simple and add TAsyncClientSocketTransport.  Fixed issues with TMessageReader and TFramedTransport.
@Jens-G
Copy link
Member

Jens-G commented Sep 23, 2015

Hi, some comments.

  • Version numbers should match the Thrift version number.

  • They also need to be mentioned in the "how to release" docs, see website, but that can be done later.

  • All (textual) files must have an ASF license header. This also includes shell scripts etc.

    Resolving dependencies... 
     + thrift 0.1.0 from path ../../../../lib/dart
     + tutorial 0.0.1 from path ../gen-dart/tutorial
    

@markerickson-wf
Copy link
Contributor

@Jens-G I'll update the versions of static Dart libraries to 0.9.3 (or whichever version you suggest). But for generated Dart libraries, such as what you're seeing in tutorial 0.0.1, it is really just an arbitrary placeholder, which developers would modify to match their own versioning scheme for a service. If there is a way to define a service version in a .thrift definition file, then I could use that, which would be ideal.

https://issues.apache.org/jira/browse/THRIFT-3299

Address code review comments to add missing License text and update version to match Thrift version.
@Jens-G
Copy link
Member

Jens-G commented Sep 24, 2015

I see, didn't recognize that the second one was generated when I wrote that. I saw it later in the compiler code. I think that's fine. Lib should be at whatever Thrift version is (the compiler tells you when called with -v) and as I said we need to document this here.

Thanks for the good, keep pushing!


Von: Mark Erickson
Gesendet: 24.09.2015 04:39
An: apache/thrift
Cc: Jens Geyer
Betreff: Re: [thrift] THRIFT-3299 Dart language bindings in Thrift (#608)

@Jens-G I'll update the versions of static Dart libraries to 0.9.3. But for generated Dart libraries, such as what you're seeing in tutorial 0.0.1, it is really just an arbitrary placeholder, which developers would modify to match their own versioning scheme for a service. If there is a way to define a service version in a .thrift definition file, then I could use that, which would be ideal.


Reply to this email directly or view it on GitHub:
#608 (comment)

@Jens-G
Copy link
Member

Jens-G commented Sep 24, 2015

No, there is no such mechanism as a version number for services. Maybe IDL annotations could be used for this, never tried that.

http://stackoverflow.com/questions/25631235/what-is-an-annotation-in-apache-thrift-and-what-is-it-used-for


Von: Mark Erickson
Gesendet: 24.09.2015 04:39
An: apache/thrift
Cc: Jens Geyer
Betreff: Re: [thrift] THRIFT-3299 Dart language bindings in Thrift (#608)

@Jens-G I'll update the versions of static Dart libraries to 0.9.3. But for generated Dart libraries, such as what you're seeing in tutorial 0.0.1, it is really just an arbitrary placeholder, which developers would modify to match their own versioning scheme for a service. If there is a way to define a service version in a .thrift definition file, then I could use that, which would be ideal.


Reply to this email directly or view it on GitHub:
#608 (comment)

@markerickson-wf
Copy link
Contributor

@Jens-G I think I have addressed your comments now. Automake files are in, cross tests are passing with a Dart client and Python server, versions and license headers are updated. I tried to identify the files that will need to be updated on release here - https://github.com/apache/thrift/pull/608/files#diff-d9d9844c5f034229628ba1f2e341ce6bR44.

@asfgit asfgit closed this in 932c470 Oct 2, 2015
allengeorge pushed a commit to allengeorge/thrift that referenced this pull request Jan 1, 2017
…ng.org).

Client: Dart
Patch: Mark Erickson <mark.erickson@workiva.com>

This closes apache#608
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants