[jira] [Resolved] (THRIFT-4285) Pull generated send/recv into library to allow behaviour to be customised

2017-11-03 Thread James E. King, III (JIRA)

 [ 
https://issues.apache.org/jira/browse/THRIFT-4285?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

James E. King, III resolved THRIFT-4285.

   Resolution: Fixed
Fix Version/s: 0.11.0

Committed - thanks everyone for your input and improvements here.

> Pull generated send/recv into library to allow behaviour to be customised
> -
>
> Key: THRIFT-4285
> URL: https://issues.apache.org/jira/browse/THRIFT-4285
> Project: Thrift
>  Issue Type: Improvement
>  Components: Go - Compiler, Go - Library
>Reporter: Chris Bannister
>Assignee: Can Celasun
>Priority: Major
> Fix For: 0.11.0
>
> Attachments: 0001-go-pull-generated-send-recv-into-lib-v6.patch, 
> 0001-go-pull-generated-send-recv-into-lib-v7.patch
>
>
> Currently it is difficult to change how thrift writes messages onto the 
> transport because they are in the generated code. Instead the generated 
> send/recv methods should be in the library. This will greatly simplify the 
> client code and remove many duplicate methods whilst allowing users more 
> flexibility to implement connection pools and other features such as THeader.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-4285) Pull generated send/recv into library to allow behaviour to be customised

2017-11-03 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16238675#comment-16238675
 ] 

ASF GitHub Bot commented on THRIFT-4285:


Github user asfgit closed the pull request at:

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


> Pull generated send/recv into library to allow behaviour to be customised
> -
>
> Key: THRIFT-4285
> URL: https://issues.apache.org/jira/browse/THRIFT-4285
> Project: Thrift
>  Issue Type: Improvement
>  Components: Go - Compiler, Go - Library
>Reporter: Chris Bannister
>Assignee: Can Celasun
>Priority: Major
> Fix For: 0.11.0
>
> Attachments: 0001-go-pull-generated-send-recv-into-lib-v6.patch, 
> 0001-go-pull-generated-send-recv-into-lib-v7.patch
>
>
> Currently it is difficult to change how thrift writes messages onto the 
> transport because they are in the generated code. Instead the generated 
> send/recv methods should be in the library. This will greatly simplify the 
> client code and remove many duplicate methods whilst allowing users more 
> flexibility to implement connection pools and other features such as THeader.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1382: THRIFT-4285 Move TX/RX methods from gen. code to ...

2017-11-03 Thread asfgit
Github user asfgit closed the pull request at:

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


---


[jira] [Commented] (THRIFT-4285) Pull generated send/recv into library to allow behaviour to be customised

2017-11-03 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16238674#comment-16238674
 ] 

ASF GitHub Bot commented on THRIFT-4285:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1382
  
Excellent, I will merge this, and we'll get it into 0.11.0 - so yay!  We 
can consider the backwards-compatible construction adapter to be deprecated so 
that it can be removed in the follow-on release.  That needs to be documented 
in the dlang readme - would you be able to document that in a separate PR?  
Since it's just a readme if you want to make it a patch on a thrift jira ticket 
that would be enough to merge in the readme change.


> Pull generated send/recv into library to allow behaviour to be customised
> -
>
> Key: THRIFT-4285
> URL: https://issues.apache.org/jira/browse/THRIFT-4285
> Project: Thrift
>  Issue Type: Improvement
>  Components: Go - Compiler, Go - Library
>Reporter: Chris Bannister
>Assignee: Can Celasun
>Priority: Major
> Attachments: 0001-go-pull-generated-send-recv-into-lib-v6.patch, 
> 0001-go-pull-generated-send-recv-into-lib-v7.patch
>
>
> Currently it is difficult to change how thrift writes messages onto the 
> transport because they are in the generated code. Instead the generated 
> send/recv methods should be in the library. This will greatly simplify the 
> client code and remove many duplicate methods whilst allowing users more 
> flexibility to implement connection pools and other features such as THeader.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift issue #1382: THRIFT-4285 Move TX/RX methods from gen. code to library

2017-11-03 Thread jeking3
Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1382
  
Excellent, I will merge this, and we'll get it into 0.11.0 - so yay!  We 
can consider the backwards-compatible construction adapter to be deprecated so 
that it can be removed in the follow-on release.  That needs to be documented 
in the dlang readme - would you be able to document that in a separate PR?  
Since it's just a readme if you want to make it a patch on a thrift jira ticket 
that would be enough to merge in the readme change.


---


[jira] [Assigned] (THRIFT-4285) Pull generated send/recv into library to allow behaviour to be customised

2017-11-03 Thread Can Celasun (JIRA)

 [ 
https://issues.apache.org/jira/browse/THRIFT-4285?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Can Celasun reassigned THRIFT-4285:
---

Assignee: Can Celasun  (was: Chris Bannister)

> Pull generated send/recv into library to allow behaviour to be customised
> -
>
> Key: THRIFT-4285
> URL: https://issues.apache.org/jira/browse/THRIFT-4285
> Project: Thrift
>  Issue Type: Improvement
>  Components: Go - Compiler, Go - Library
>Reporter: Chris Bannister
>Assignee: Can Celasun
>Priority: Major
> Attachments: 0001-go-pull-generated-send-recv-into-lib-v6.patch, 
> 0001-go-pull-generated-send-recv-into-lib-v7.patch
>
>
> Currently it is difficult to change how thrift writes messages onto the 
> transport because they are in the generated code. Instead the generated 
> send/recv methods should be in the library. This will greatly simplify the 
> client code and remove many duplicate methods whilst allowing users more 
> flexibility to implement connection pools and other features such as THeader.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-4285) Pull generated send/recv into library to allow behaviour to be customised

2017-11-03 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16238360#comment-16238360
 ] 

ASF GitHub Bot commented on THRIFT-4285:


Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1382
  
@jeking3 All done! I think we are good to merge, what do you think?


> Pull generated send/recv into library to allow behaviour to be customised
> -
>
> Key: THRIFT-4285
> URL: https://issues.apache.org/jira/browse/THRIFT-4285
> Project: Thrift
>  Issue Type: Improvement
>  Components: Go - Compiler, Go - Library
>Reporter: Chris Bannister
>Assignee: Chris Bannister
>Priority: Major
> Attachments: 0001-go-pull-generated-send-recv-into-lib-v6.patch, 
> 0001-go-pull-generated-send-recv-into-lib-v7.patch
>
>
> Currently it is difficult to change how thrift writes messages onto the 
> transport because they are in the generated code. Instead the generated 
> send/recv methods should be in the library. This will greatly simplify the 
> client code and remove many duplicate methods whilst allowing users more 
> flexibility to implement connection pools and other features such as THeader.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift issue #1382: THRIFT-4285 Move TX/RX methods from gen. code to library

2017-11-03 Thread dcelasun
Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1382
  
@jeking3 All done! I think we are good to merge, what do you think?


---


[jira] [Commented] (THRIFT-4285) Pull generated send/recv into library to allow behaviour to be customised

2017-11-03 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16237713#comment-16237713
 ] 

ASF GitHub Bot commented on THRIFT-4285:


Github user dcelasun commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1382#discussion_r148808675
  
--- Diff: lib/go/test/tests/client_error_test.go ---
@@ -411,7 +411,7 @@ func TestClientReportTTransportErrors(t *testing.T) {
if !prepareClientCallReply(protocol, i, err) {
return
}
-   client := errortest.NewErrorTestClientProtocol(transport, 
protocol, protocol)
+   client := 
errortest.NewErrorTestClient(thrift.NewTStandardClient(protocol, protocol))
--- End diff --

Good point :) I'll add both, but it'll have to wait till tomorrow as I've 
already left the office.


> Pull generated send/recv into library to allow behaviour to be customised
> -
>
> Key: THRIFT-4285
> URL: https://issues.apache.org/jira/browse/THRIFT-4285
> Project: Thrift
>  Issue Type: Improvement
>  Components: Go - Compiler, Go - Library
>Reporter: Chris Bannister
>Assignee: Chris Bannister
>Priority: Major
> Attachments: 0001-go-pull-generated-send-recv-into-lib-v6.patch, 
> 0001-go-pull-generated-send-recv-into-lib-v7.patch
>
>
> Currently it is difficult to change how thrift writes messages onto the 
> transport because they are in the generated code. Instead the generated 
> send/recv methods should be in the library. This will greatly simplify the 
> client code and remove many duplicate methods whilst allowing users more 
> flexibility to implement connection pools and other features such as THeader.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-4285) Pull generated send/recv into library to allow behaviour to be customised

2017-11-03 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16237714#comment-16237714
 ] 

ASF GitHub Bot commented on THRIFT-4285:


Github user dcelasun commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1382#discussion_r148808724
  
--- Diff: build/docker/ubuntu-trusty/Dockerfile ---
@@ -221,3 +221,4 @@ ENV THRIFT_ROOT /thrift
 RUN mkdir -p $THRIFT_ROOT/src
 COPY Dockerfile $THRIFT_ROOT/
 WORKDIR $THRIFT_ROOT/src
+
--- End diff --

My bad, will revert this as well.


> Pull generated send/recv into library to allow behaviour to be customised
> -
>
> Key: THRIFT-4285
> URL: https://issues.apache.org/jira/browse/THRIFT-4285
> Project: Thrift
>  Issue Type: Improvement
>  Components: Go - Compiler, Go - Library
>Reporter: Chris Bannister
>Assignee: Chris Bannister
>Priority: Major
> Attachments: 0001-go-pull-generated-send-recv-into-lib-v6.patch, 
> 0001-go-pull-generated-send-recv-into-lib-v7.patch
>
>
> Currently it is difficult to change how thrift writes messages onto the 
> transport because they are in the generated code. Instead the generated 
> send/recv methods should be in the library. This will greatly simplify the 
> client code and remove many duplicate methods whilst allowing users more 
> flexibility to implement connection pools and other features such as THeader.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1382: THRIFT-4285 Move TX/RX methods from gen. code to ...

2017-11-03 Thread dcelasun
Github user dcelasun commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1382#discussion_r148808724
  
--- Diff: build/docker/ubuntu-trusty/Dockerfile ---
@@ -221,3 +221,4 @@ ENV THRIFT_ROOT /thrift
 RUN mkdir -p $THRIFT_ROOT/src
 COPY Dockerfile $THRIFT_ROOT/
 WORKDIR $THRIFT_ROOT/src
+
--- End diff --

My bad, will revert this as well.


---


[jira] [Commented] (THRIFT-4285) Pull generated send/recv into library to allow behaviour to be customised

2017-11-03 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16237712#comment-16237712
 ] 

ASF GitHub Bot commented on THRIFT-4285:


Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1382#discussion_r148808549
  
--- Diff: build/docker/ubuntu-trusty/Dockerfile ---
@@ -221,3 +221,4 @@ ENV THRIFT_ROOT /thrift
 RUN mkdir -p $THRIFT_ROOT/src
 COPY Dockerfile $THRIFT_ROOT/
 WORKDIR $THRIFT_ROOT/src
+
--- End diff --

This extra line will force an image rebuild.


> Pull generated send/recv into library to allow behaviour to be customised
> -
>
> Key: THRIFT-4285
> URL: https://issues.apache.org/jira/browse/THRIFT-4285
> Project: Thrift
>  Issue Type: Improvement
>  Components: Go - Compiler, Go - Library
>Reporter: Chris Bannister
>Assignee: Chris Bannister
>Priority: Major
> Attachments: 0001-go-pull-generated-send-recv-into-lib-v6.patch, 
> 0001-go-pull-generated-send-recv-into-lib-v7.patch
>
>
> Currently it is difficult to change how thrift writes messages onto the 
> transport because they are in the generated code. Instead the generated 
> send/recv methods should be in the library. This will greatly simplify the 
> client code and remove many duplicate methods whilst allowing users more 
> flexibility to implement connection pools and other features such as THeader.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1382: THRIFT-4285 Move TX/RX methods from gen. code to ...

2017-11-03 Thread dcelasun
Github user dcelasun commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1382#discussion_r148808675
  
--- Diff: lib/go/test/tests/client_error_test.go ---
@@ -411,7 +411,7 @@ func TestClientReportTTransportErrors(t *testing.T) {
if !prepareClientCallReply(protocol, i, err) {
return
}
-   client := errortest.NewErrorTestClientProtocol(transport, 
protocol, protocol)
+   client := 
errortest.NewErrorTestClient(thrift.NewTStandardClient(protocol, protocol))
--- End diff --

Good point :) I'll add both, but it'll have to wait till tomorrow as I've 
already left the office.


---


[GitHub] thrift pull request #1382: THRIFT-4285 Move TX/RX methods from gen. code to ...

2017-11-03 Thread jeking3
Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1382#discussion_r148808549
  
--- Diff: build/docker/ubuntu-trusty/Dockerfile ---
@@ -221,3 +221,4 @@ ENV THRIFT_ROOT /thrift
 RUN mkdir -p $THRIFT_ROOT/src
 COPY Dockerfile $THRIFT_ROOT/
 WORKDIR $THRIFT_ROOT/src
+
--- End diff --

This extra line will force an image rebuild.


---


[jira] [Commented] (THRIFT-4285) Pull generated send/recv into library to allow behaviour to be customised

2017-11-03 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16237709#comment-16237709
 ] 

ASF GitHub Bot commented on THRIFT-4285:


Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1382#discussion_r148808038
  
--- Diff: lib/go/test/tests/client_error_test.go ---
@@ -411,7 +411,7 @@ func TestClientReportTTransportErrors(t *testing.T) {
if !prepareClientCallReply(protocol, i, err) {
return
}
-   client := errortest.NewErrorTestClientProtocol(transport, 
protocol, protocol)
+   client := 
errortest.NewErrorTestClient(thrift.NewTStandardClient(protocol, protocol))
--- End diff --

Not having to change the test is a better proof that the original method 
works :)


> Pull generated send/recv into library to allow behaviour to be customised
> -
>
> Key: THRIFT-4285
> URL: https://issues.apache.org/jira/browse/THRIFT-4285
> Project: Thrift
>  Issue Type: Improvement
>  Components: Go - Compiler, Go - Library
>Reporter: Chris Bannister
>Assignee: Chris Bannister
>Priority: Major
> Attachments: 0001-go-pull-generated-send-recv-into-lib-v6.patch, 
> 0001-go-pull-generated-send-recv-into-lib-v7.patch
>
>
> Currently it is difficult to change how thrift writes messages onto the 
> transport because they are in the generated code. Instead the generated 
> send/recv methods should be in the library. This will greatly simplify the 
> client code and remove many duplicate methods whilst allowing users more 
> flexibility to implement connection pools and other features such as THeader.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1382: THRIFT-4285 Move TX/RX methods from gen. code to ...

2017-11-03 Thread jeking3
Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1382#discussion_r148808038
  
--- Diff: lib/go/test/tests/client_error_test.go ---
@@ -411,7 +411,7 @@ func TestClientReportTTransportErrors(t *testing.T) {
if !prepareClientCallReply(protocol, i, err) {
return
}
-   client := errortest.NewErrorTestClientProtocol(transport, 
protocol, protocol)
+   client := 
errortest.NewErrorTestClient(thrift.NewTStandardClient(protocol, protocol))
--- End diff --

Not having to change the test is a better proof that the original method 
works :)


---


[jira] [Commented] (THRIFT-4285) Pull generated send/recv into library to allow behaviour to be customised

2017-11-03 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16237666#comment-16237666
 ] 

ASF GitHub Bot commented on THRIFT-4285:


Github user dcelasun commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1382#discussion_r148798731
  
--- Diff: build/docker/ubuntu-trusty/Dockerfile ---
@@ -217,6 +217,62 @@ RUN rm -rf /var/cache/apt/* && \
 rm -rf /tmp/* && \
 rm -rf /var/tmp/*
 
+# Ruby
--- End diff --

Fixed.


> Pull generated send/recv into library to allow behaviour to be customised
> -
>
> Key: THRIFT-4285
> URL: https://issues.apache.org/jira/browse/THRIFT-4285
> Project: Thrift
>  Issue Type: Improvement
>  Components: Go - Compiler, Go - Library
>Reporter: Chris Bannister
>Assignee: Chris Bannister
>Priority: Major
> Attachments: 0001-go-pull-generated-send-recv-into-lib-v6.patch, 
> 0001-go-pull-generated-send-recv-into-lib-v7.patch
>
>
> Currently it is difficult to change how thrift writes messages onto the 
> transport because they are in the generated code. Instead the generated 
> send/recv methods should be in the library. This will greatly simplify the 
> client code and remove many duplicate methods whilst allowing users more 
> flexibility to implement connection pools and other features such as THeader.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1382: THRIFT-4285 Move TX/RX methods from gen. code to ...

2017-11-03 Thread dcelasun
Github user dcelasun commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1382#discussion_r148798731
  
--- Diff: build/docker/ubuntu-trusty/Dockerfile ---
@@ -217,6 +217,62 @@ RUN rm -rf /var/cache/apt/* && \
 rm -rf /tmp/* && \
 rm -rf /var/tmp/*
 
+# Ruby
--- End diff --

Fixed.


---


[GitHub] thrift issue #1410: Common Lisp support

2017-11-03 Thread dkochmanski
Github user dkochmanski commented on the issue:

https://github.com/apache/thrift/pull/1410
  
Thank you for taking your time for the review


---


[jira] [Commented] (THRIFT-4285) Pull generated send/recv into library to allow behaviour to be customised

2017-11-03 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16237661#comment-16237661
 ] 

ASF GitHub Bot commented on THRIFT-4285:


Github user dcelasun commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1382#discussion_r148797018
  
--- Diff: lib/go/test/tests/client_error_test.go ---
@@ -411,7 +411,7 @@ func TestClientReportTTransportErrors(t *testing.T) {
if !prepareClientCallReply(protocol, i, err) {
return
}
-   client := errortest.NewErrorTestClientProtocol(transport, 
protocol, protocol)
+   client := 
errortest.NewErrorTestClient(thrift.NewTStandardClient(protocol, protocol))
--- End diff --

No, it's backwards compatible now (see 
[here](https://github.com/dcelasun/thrift/blob/555efe5aefe9619a900471e56e86906d40bc96b9/compiler/cpp/src/thrift/generate/t_go_generator.cc#L1889-L1930)),
 the test simply uses the new method.

I've also tested it with the integration of tests of a real, production 
app. It works as expected.


> Pull generated send/recv into library to allow behaviour to be customised
> -
>
> Key: THRIFT-4285
> URL: https://issues.apache.org/jira/browse/THRIFT-4285
> Project: Thrift
>  Issue Type: Improvement
>  Components: Go - Compiler, Go - Library
>Reporter: Chris Bannister
>Assignee: Chris Bannister
>Priority: Major
> Attachments: 0001-go-pull-generated-send-recv-into-lib-v6.patch, 
> 0001-go-pull-generated-send-recv-into-lib-v7.patch
>
>
> Currently it is difficult to change how thrift writes messages onto the 
> transport because they are in the generated code. Instead the generated 
> send/recv methods should be in the library. This will greatly simplify the 
> client code and remove many duplicate methods whilst allowing users more 
> flexibility to implement connection pools and other features such as THeader.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1410: Common Lisp support

2017-11-03 Thread dkochmanski
Github user dkochmanski commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1410#discussion_r148797236
  
--- Diff: lib/cl/framed-transport.lisp ---
@@ -0,0 +1,136 @@
+(in-package #:org.apache.thrift.implementation)
+
+ Copyright 2017 Rigetti Computing 
--- End diff --

Our work on Thrift is sponsored by Rigetti, so there is no need for that.


---


[GitHub] thrift pull request #1382: THRIFT-4285 Move TX/RX methods from gen. code to ...

2017-11-03 Thread dcelasun
Github user dcelasun commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1382#discussion_r148797018
  
--- Diff: lib/go/test/tests/client_error_test.go ---
@@ -411,7 +411,7 @@ func TestClientReportTTransportErrors(t *testing.T) {
if !prepareClientCallReply(protocol, i, err) {
return
}
-   client := errortest.NewErrorTestClientProtocol(transport, 
protocol, protocol)
+   client := 
errortest.NewErrorTestClient(thrift.NewTStandardClient(protocol, protocol))
--- End diff --

No, it's backwards compatible now (see 
[here](https://github.com/dcelasun/thrift/blob/555efe5aefe9619a900471e56e86906d40bc96b9/compiler/cpp/src/thrift/generate/t_go_generator.cc#L1889-L1930)),
 the test simply uses the new method.

I've also tested it with the integration of tests of a real, production 
app. It works as expected.


---


[GitHub] thrift pull request #1410: Common Lisp support

2017-11-03 Thread dkochmanski
Github user dkochmanski commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1410#discussion_r148796969
  
--- Diff: lib/cl/externals/bundle-info.sexp ---
@@ -0,0 +1,14 @@
+(:CREATION-TIME #A((20) BASE-CHAR . "2017-10-31T11:49:23Z") 
:REQUESTED-SYSTEMS
--- End diff --

all things put in `externals` are checked in, so whole directory will be 
removed (this file included).


---


[jira] [Commented] (THRIFT-4285) Pull generated send/recv into library to allow behaviour to be customised

2017-11-03 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16237657#comment-16237657
 ] 

ASF GitHub Bot commented on THRIFT-4285:


Github user dcelasun commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1382#discussion_r148796351
  
--- Diff: build/docker/ubuntu-trusty/Dockerfile ---
@@ -217,6 +217,62 @@ RUN rm -rf /var/cache/apt/* && \
 rm -rf /tmp/* && \
 rm -rf /var/tmp/*
 
+# Ruby
--- End diff --

It's because I messed up the rebase. Will fix.


> Pull generated send/recv into library to allow behaviour to be customised
> -
>
> Key: THRIFT-4285
> URL: https://issues.apache.org/jira/browse/THRIFT-4285
> Project: Thrift
>  Issue Type: Improvement
>  Components: Go - Compiler, Go - Library
>Reporter: Chris Bannister
>Assignee: Chris Bannister
>Priority: Major
> Attachments: 0001-go-pull-generated-send-recv-into-lib-v6.patch, 
> 0001-go-pull-generated-send-recv-into-lib-v7.patch
>
>
> Currently it is difficult to change how thrift writes messages onto the 
> transport because they are in the generated code. Instead the generated 
> send/recv methods should be in the library. This will greatly simplify the 
> client code and remove many duplicate methods whilst allowing users more 
> flexibility to implement connection pools and other features such as THeader.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1382: THRIFT-4285 Move TX/RX methods from gen. code to ...

2017-11-03 Thread dcelasun
Github user dcelasun commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1382#discussion_r148796351
  
--- Diff: build/docker/ubuntu-trusty/Dockerfile ---
@@ -217,6 +217,62 @@ RUN rm -rf /var/cache/apt/* && \
 rm -rf /tmp/* && \
 rm -rf /var/tmp/*
 
+# Ruby
--- End diff --

It's because I messed up the rebase. Will fix.


---


[jira] [Commented] (THRIFT-4285) Pull generated send/recv into library to allow behaviour to be customised

2017-11-03 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16237645#comment-16237645
 ] 

ASF GitHub Bot commented on THRIFT-4285:


Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1382#discussion_r148793389
  
--- Diff: lib/go/test/tests/client_error_test.go ---
@@ -411,7 +411,7 @@ func TestClientReportTTransportErrors(t *testing.T) {
if !prepareClientCallReply(protocol, i, err) {
return
}
-   client := errortest.NewErrorTestClientProtocol(transport, 
protocol, protocol)
+   client := 
errortest.NewErrorTestClient(thrift.NewTStandardClient(protocol, protocol))
--- End diff --

Does this means the change is still not backwards compatible with 
previously written customer code?


> Pull generated send/recv into library to allow behaviour to be customised
> -
>
> Key: THRIFT-4285
> URL: https://issues.apache.org/jira/browse/THRIFT-4285
> Project: Thrift
>  Issue Type: Improvement
>  Components: Go - Compiler, Go - Library
>Reporter: Chris Bannister
>Assignee: Chris Bannister
>Priority: Major
> Attachments: 0001-go-pull-generated-send-recv-into-lib-v6.patch, 
> 0001-go-pull-generated-send-recv-into-lib-v7.patch
>
>
> Currently it is difficult to change how thrift writes messages onto the 
> transport because they are in the generated code. Instead the generated 
> send/recv methods should be in the library. This will greatly simplify the 
> client code and remove many duplicate methods whilst allowing users more 
> flexibility to implement connection pools and other features such as THeader.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-4285) Pull generated send/recv into library to allow behaviour to be customised

2017-11-03 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16237646#comment-16237646
 ] 

ASF GitHub Bot commented on THRIFT-4285:


Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1382#discussion_r148793005
  
--- Diff: build/docker/ubuntu-trusty/Dockerfile ---
@@ -217,6 +217,62 @@ RUN rm -rf /var/cache/apt/* && \
 rm -rf /tmp/* && \
 rm -rf /var/tmp/*
 
+# Ruby
--- End diff --

This change looks incorrect.  It duplicates other parts of the file.


> Pull generated send/recv into library to allow behaviour to be customised
> -
>
> Key: THRIFT-4285
> URL: https://issues.apache.org/jira/browse/THRIFT-4285
> Project: Thrift
>  Issue Type: Improvement
>  Components: Go - Compiler, Go - Library
>Reporter: Chris Bannister
>Assignee: Chris Bannister
>Priority: Major
> Attachments: 0001-go-pull-generated-send-recv-into-lib-v6.patch, 
> 0001-go-pull-generated-send-recv-into-lib-v7.patch
>
>
> Currently it is difficult to change how thrift writes messages onto the 
> transport because they are in the generated code. Instead the generated 
> send/recv methods should be in the library. This will greatly simplify the 
> client code and remove many duplicate methods whilst allowing users more 
> flexibility to implement connection pools and other features such as THeader.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1382: THRIFT-4285 Move TX/RX methods from gen. code to ...

2017-11-03 Thread jeking3
Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1382#discussion_r148793389
  
--- Diff: lib/go/test/tests/client_error_test.go ---
@@ -411,7 +411,7 @@ func TestClientReportTTransportErrors(t *testing.T) {
if !prepareClientCallReply(protocol, i, err) {
return
}
-   client := errortest.NewErrorTestClientProtocol(transport, 
protocol, protocol)
+   client := 
errortest.NewErrorTestClient(thrift.NewTStandardClient(protocol, protocol))
--- End diff --

Does this means the change is still not backwards compatible with 
previously written customer code?


---


[GitHub] thrift pull request #1382: THRIFT-4285 Move TX/RX methods from gen. code to ...

2017-11-03 Thread jeking3
Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1382#discussion_r148793005
  
--- Diff: build/docker/ubuntu-trusty/Dockerfile ---
@@ -217,6 +217,62 @@ RUN rm -rf /var/cache/apt/* && \
 rm -rf /tmp/* && \
 rm -rf /var/tmp/*
 
+# Ruby
--- End diff --

This change looks incorrect.  It duplicates other parts of the file.


---


[jira] [Assigned] (THRIFT-82) Common Lisp support

2017-11-03 Thread James E. King, III (JIRA)

 [ 
https://issues.apache.org/jira/browse/THRIFT-82?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

James E. King, III reassigned THRIFT-82:


Assignee: James E. King, III

> Common Lisp support
> ---
>
> Key: THRIFT-82
> URL: https://issues.apache.org/jira/browse/THRIFT-82
> Project: Thrift
>  Issue Type: New Feature
>Reporter: Patrick Collison
>Assignee: James E. King, III
>Priority: Major
> Attachments: thrift-cl.patch
>
>
> Common Lisp support is attached



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-82) Common Lisp support

2017-11-03 Thread James E. King, III (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-82?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16237642#comment-16237642
 ] 

James E. King, III commented on THRIFT-82:
--

[~jfarrell] consider adding "Common Lisp - Compiler" and "Common Lisp - 
Library" to the components list when finished.

> Common Lisp support
> ---
>
> Key: THRIFT-82
> URL: https://issues.apache.org/jira/browse/THRIFT-82
> Project: Thrift
>  Issue Type: New Feature
>Reporter: Patrick Collison
>Priority: Major
> Attachments: thrift-cl.patch
>
>
> Common Lisp support is attached



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1410: Common Lisp support

2017-11-03 Thread jeking3
Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1410#discussion_r148790161
  
--- Diff: lib/cl/externals/bundle.lisp ---
@@ -0,0 +1,161 @@
+(cl:in-package #:cl-user)
--- End diff --

Missing license statement.


---


[GitHub] thrift pull request #1410: Common Lisp support

2017-11-03 Thread jeking3
Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1410#discussion_r148790890
  
--- Diff: 
lib/cl/externals/software/usocket-0.7.0.1/vendor/OpenTransportUDP.lisp ---
@@ -0,0 +1,146 @@
+;;;-*-Mode: LISP; Package: CCL -*-
--- End diff --

As with the other libraries, this should not be checked in.  I have skipped 
over all of the externals/ due to this.


---


[GitHub] thrift pull request #1410: Common Lisp support

2017-11-03 Thread jeking3
Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1410#discussion_r148791102
  
--- Diff: lib/cl/framed-transport.lisp ---
@@ -0,0 +1,136 @@
+(in-package #:org.apache.thrift.implementation)
+
+ Copyright 2017 Rigetti Computing 
--- End diff --

@jfarrell do we need to check with them to make sure this is okay to submit?


---


[GitHub] thrift pull request #1410: Common Lisp support

2017-11-03 Thread jeking3
Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1410#discussion_r148790668
  
--- Diff: lib/cl/externals/software/bordeaux-threads-v0.8.5/.travis.yml ---
@@ -0,0 +1,44 @@
+language: lisp
--- End diff --

If you checked in bordeaux-threads, it should not be part of the 
submission.  Instead, the build process should install it on demand in the same 
manner that the nodejs implementation calls npm to install packages during the 
build.


---


[GitHub] thrift pull request #1410: Common Lisp support

2017-11-03 Thread jeking3
Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1410#discussion_r148791220
  
--- Diff: lib/cl/binary-protocol.lisp ---
@@ -0,0 +1,255 @@
+(in-package #:org.apache.thrift.implementation)
+
+ This file defines the concrete `binary-protocol` layer for the 
`org.apache.thrift` library.
+
+ copyright 2010 [james anderson](james.ander...@setf.de)
--- End diff --

@jfarrell do we need to check with them to make sure this is okay to submit?


---


[GitHub] thrift pull request #1410: Common Lisp support

2017-11-03 Thread jeking3
Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1410#discussion_r148790488
  
--- Diff: lib/cl/externals/software/alexandria-20170830-git/.boring ---
@@ -0,0 +1,13 @@
+# Boring file regexps:
--- End diff --

If you checked in alexandria, it should not be part of the submission.  
Instead, the build process should install it on demand in the same manner that 
the nodejs implementation calls npm to install packages during the build.


---


[GitHub] thrift pull request #1410: Common Lisp support

2017-11-03 Thread jeking3
Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1410#discussion_r148789088
  
--- Diff: compiler/cpp/src/thrift/generate/t_cl_generator.cc ---
@@ -0,0 +1,544 @@
+// Copyright (c) 2008- Patrick Collison 
--- End diff --

This needs the standard Apache Software License statement applied.


---


[GitHub] thrift issue #1410: Common Lisp support

2017-11-03 Thread jeking3
Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1410
  
@jfarrell for some reason there is no recorded Travis CI build for this 
pull request.  When the author squashes and resubmits hopefully this will kick 
off a proper build?


---


[jira] [Commented] (THRIFT-82) Common Lisp support

2017-11-03 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-82?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16237624#comment-16237624
 ] 

ASF GitHub Bot commented on THRIFT-82:
--

Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1410
  
Some initial thoughts, before I review all of the files individually:

1. Please squash.
2. Please add [THRIFT-82] at the beginning of the commit description and 
the pull request description.
3. Please review https://thrift.apache.org/docs/HowToContribute.

I resurrected https://issues.apache.org/jira/browse/THRIFT-82 upon seeing 
this.  I'll start going through the files.


> Common Lisp support
> ---
>
> Key: THRIFT-82
> URL: https://issues.apache.org/jira/browse/THRIFT-82
> Project: Thrift
>  Issue Type: New Feature
>Reporter: Patrick Collison
>Priority: Major
> Attachments: thrift-cl.patch
>
>
> Common Lisp support is attached



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift issue #1410: Common Lisp support

2017-11-03 Thread jeking3
Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1410
  
Some initial thoughts, before I review all of the files individually:

1. Please squash.
2. Please add [THRIFT-82] at the beginning of the commit description and 
the pull request description.
3. Please review https://thrift.apache.org/docs/HowToContribute.

I resurrected https://issues.apache.org/jira/browse/THRIFT-82 upon seeing 
this.  I'll start going through the files.


---


[jira] [Commented] (THRIFT-82) Common Lisp support

2017-11-03 Thread James E. King, III (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-82?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16237622#comment-16237622
 ] 

James E. King, III commented on THRIFT-82:
--

A pull request was submitted today implementing Common Lisp support so I am 
reopening this.
https://github.com/apache/thrift/pull/1410

> Common Lisp support
> ---
>
> Key: THRIFT-82
> URL: https://issues.apache.org/jira/browse/THRIFT-82
> Project: Thrift
>  Issue Type: New Feature
>Reporter: Patrick Collison
>Priority: Major
> Attachments: thrift-cl.patch
>
>
> Common Lisp support is attached



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Reopened] (THRIFT-82) Common Lisp support

2017-11-03 Thread James E. King, III (JIRA)

 [ 
https://issues.apache.org/jira/browse/THRIFT-82?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

James E. King, III reopened THRIFT-82:
--

> Common Lisp support
> ---
>
> Key: THRIFT-82
> URL: https://issues.apache.org/jira/browse/THRIFT-82
> Project: Thrift
>  Issue Type: New Feature
>Reporter: Patrick Collison
>Priority: Major
> Attachments: thrift-cl.patch
>
>
> Common Lisp support is attached



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1410: Common Lisp support

2017-11-03 Thread uint
GitHub user uint opened a pull request:

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

Common Lisp support

This adds Common Lisp support - library, generator, cross-tests, tutorial 
code, etc. Currently only SBCL is supported.

The minimal requirements are fulfilled. Cross-tests pass against most 
languages that I've tried.

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

$ git pull https://github.com/TurtleWarePL/thrift develop

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

https://github.com/apache/thrift/pull/1410.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 #1410


commit 171e9a60784910d48006f1c1c4e08b64f81b22b4
Author: Tomek Kurcz 
Date:   2017-09-19T07:16:43Z

Patch Thrift with de.setf.thrift

commit d4850239d69d32a11793152f9cb98b46bea55075
Author: Tomek Kurcz 
Date:   2017-09-19T07:16:56Z

CL generator: fix and integrate it

commit a48a65de910b8769c3754bd8bdfe69ba1f0e9bc0
Author: Tomek Kurcz 
Date:   2017-09-19T08:42:55Z

Remove non-existent packages and system dependencies

commit 1fae86f6c77bb82847607507f68b92907385b325
Author: Tomek Kurcz 
Date:   2017-09-19T10:05:54Z

Add namespace declarations for CL in tutorial .thrift files

commit 6668f0984d936539ee13755bbdfb57f6cbe760e6
Author: Tomek Kurcz 
Date:   2017-09-19T12:53:39Z

Fix Thrift URIs

commit f85eab281eb89d43dda5eb8cf869cbe23690b008
Author: Tomek Kurcz 
Date:   2017-09-19T13:20:10Z

Cosmetic: Remove emacs file headers

commit a6f873e14ee33c5229e40a3101f4732d87b4caf7
Author: Tomek Kurcz 
Date:   2017-09-20T10:35:06Z

Fix the handling of Thrift types

The decoder should expect i32 when the field type is enum

Rename i08 to i8 according to Thrift's expectations

commit b6c44618c3776e9d5ce80e9276b9e176b7fb96f9
Author: Tomek Kurcz 
Date:   2017-09-21T06:39:10Z

Defined services should just be exported to the current *package*

commit 6393e7d75cec8822d6f57d4cec14a6563528
Author: Tomek Kurcz 
Date:   2017-09-21T07:38:19Z

Bugfix: wrong order of arguments in a function call

commit 8cafef222a8a8f532627a7f31c5f5a7568954283
Author: Tomek Kurcz 
Date:   2017-09-22T09:17:31Z

Ensure users can use :common-lisp symbols when implementing services

commit c413c64ff7699151916a01638c1f02d37b473acc
Author: Tomek Kurcz 
Date:   2017-09-22T11:29:55Z

CL generator: Generate ASDF systems for Thrift programs

Also adds the CLI option not to generate .asd files, but the default is to
generate them.

commit cf85ca14704c51aeafd926bc4cc14f6fdbdd3117
Author: Tomek Kurcz 
Date:   2017-09-22T11:50:10Z

Don't expect server implementation to exist when loading gen'd code

We probably expect those to be declared later

commit 11bdf23b208dad9c2a95d0e1350782c75b4b1bb6
Author: Tomek Kurcz 
Date:   2017-09-25T09:50:26Z

CL generator: Put generated "programs" in separate directories

commit 19ff832d21599618c389fba44963f519c4637712
Author: Tomek Kurcz 
Date:   2017-09-25T11:20:14Z

CL generator: copy the options string to comments in generated files

commit cfdbc1e2ad1bd1d8a66888a0db17879c9cae0562
Author: Tomek Kurcz 
Date:   2017-09-25T11:33:02Z

CL generator: Fix the remaining warnings

commit 340486f190a9cc66555dbdaac6c6ee463bfb615a
Author: Tomek Kurcz 
Date:   2017-09-25T12:08:39Z

Replace the float conversion code with `ieee-floats` from quicklisp

commit c990ad183817e1199aa222afa38996c7dd405e0e
Author: Tomek Kurcz 
Date:   2017-09-26T08:25:46Z

CL generator: Add the option to change the ASDF system prefix

Also: Cosmetic: add a newline after a generated (def-service)

commit fb63de742ccf20d4324cdf20ded1aeca7c81dde4
Author: Tomek Kurcz 
Date:   2017-09-27T08:32:52Z

Fix the load order of thrift-test components

commit 2e0fb30a62f1881fc6cb3946cff20e5c46096f4a
Author: Tomek Kurcz 
Date:   2017-09-27T08:33:17Z

Export vector-stream for use in thrift-test

commit 80494401afcc52bbb5e6d40bcd374986078d195c
Author: Tomek Kurcz 
Date:   2017-09-27T08:38:43Z

Exclude definition-operators.lisp from compilation for now

commit f847cf503dc2cba7f3efb34bef04793d0a068a3c
Author: Tomek Kurcz 
Date:   2017-09-27T08:53:34Z

Cosmetic: Typos and reindentation

commit 61c60b4d3b3c76ce46868b4e9c2a949cf5c4fc4c
Author: Tomek Kurcz 
Date:   2017-09-27T09:01:14Z

Fix vector-protocol.write-sequence

commit