[jira] [Commented] (THRIFT-4447) Golang: Panic on p.c.Call when using deprecated initializers

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4447:


Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1461
  
Exactly.


> Golang: Panic on p.c.Call when using deprecated initializers
> 
>
> Key: THRIFT-4447
> URL: https://issues.apache.org/jira/browse/THRIFT-4447
> Project: Thrift
>  Issue Type: Bug
>  Components: Go - Compiler
>Affects Versions: 0.11.0
>Reporter: John Boiles
>
> Latest thrift:master can cause panics when using deprecated 
> `New*ClientFactory` and `New*ClientProtocol` functions. This happens because 
> both the Client and the BaseClient have an instance of a {{thrift.TClient}}. 
> The deprecated methods initialize the BaseClient's TClient, but other methods 
> use the Client's {{TClient}}.
> For example, current thrift master generates structs like this
> {code}
> type MyServiceClient struct {
>   c thrift.TClient
>   *MyServiceBaseClient
> }
> type MyServiceBaseClient struct {
>   c thrift.TClient
> }
> {code}
> And also a method like this:
> {code}
> func NewMyServiceClientFactory(t thrift.TTransport, f 
> thrift.TProtocolFactory) *MyServiceClient {
>   return {MyServiceBaseClient: 
> NewMyServiceBaseClientFactory(t, f)}
> }
> {code}
> If that method is used, later calls to service methods will panic, since 
> {{p.c}} is nil (the actual client was stored in {{p.BaseMyServiceClient.c}}).
> {code}
> func (p *MyServiceClient) DoStuff(ctx context.Context, request 
> *DoStuffRequest) (r *DoStuffResponse, err error) {
>   var _args139 DoStuffArgs
>   _args139.Request = request
>   var _result140 DoStuffResult
>   if err = p.c.Call(ctx, "do_stuff", &_args139, &_result140); err != nil 
> { // PANIC
>   ...
> {code}
> In progress fix here :https://github.com/apache/thrift/pull/1461. The fix in 
> this PR merely sets both instances of {{TClient}} (which is what happens in 
> the non-deprecated {{New*Client}} function).
> This patch currently fails {{make -k check}} however, since 
> {{src/tutorial/tutorial.go}} tries to access a different package's version of 
> the BaseClient.
> {code}
> src/tutorial/tutorial.go:477:33: bc.c undefined (cannot refer to unexported 
> field or method c)
> {code}
> The fix for that test could possibly be to expose the BaseClient's instance 
> of {{c}} (by making it a capital and thus exported {{C}}), or adding an 
> accessor method {{C()}} or {{Client()}}.
> Possibly a better fix would be to either remove these deprecated methods, or 
> figure out which {{TClient}} is the correct one to set.
> I'm not sure which is preferred, so I'm hoping to get some feedback/input 
> here.



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

[GitHub] thrift issue #1461: THRIFT-4447: Golang: Fix panic on p.c.Call when using de...

[dcelasun]

Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1461
  
Exactly.


---

[GitHub] thrift issue #1461: THRIFT-4447: Golang: Fix panic on p.c.Call when using de...

[johnboiles]

Github user johnboiles commented on the issue:

https://github.com/apache/thrift/pull/1461
  
Good call, so then:

```
func (p *BaseClient) Client_() thrift.TClient {
return p.c
}
```
?


---

[jira] [Commented] (THRIFT-4447) Golang: Panic on p.c.Call when using deprecated initializers

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4447:


Github user johnboiles commented on the issue:

https://github.com/apache/thrift/pull/1461
  
Good call, so then:

```
func (p *BaseClient) Client_() thrift.TClient {
return p.c
}
```
?


> Golang: Panic on p.c.Call when using deprecated initializers
> 
>
> Key: THRIFT-4447
> URL: https://issues.apache.org/jira/browse/THRIFT-4447
> Project: Thrift
>  Issue Type: Bug
>  Components: Go - Compiler
>Affects Versions: 0.11.0
>Reporter: John Boiles
>
> Latest thrift:master can cause panics when using deprecated 
> `New*ClientFactory` and `New*ClientProtocol` functions. This happens because 
> both the Client and the BaseClient have an instance of a {{thrift.TClient}}. 
> The deprecated methods initialize the BaseClient's TClient, but other methods 
> use the Client's {{TClient}}.
> For example, current thrift master generates structs like this
> {code}
> type MyServiceClient struct {
>   c thrift.TClient
>   *MyServiceBaseClient
> }
> type MyServiceBaseClient struct {
>   c thrift.TClient
> }
> {code}
> And also a method like this:
> {code}
> func NewMyServiceClientFactory(t thrift.TTransport, f 
> thrift.TProtocolFactory) *MyServiceClient {
>   return {MyServiceBaseClient: 
> NewMyServiceBaseClientFactory(t, f)}
> }
> {code}
> If that method is used, later calls to service methods will panic, since 
> {{p.c}} is nil (the actual client was stored in {{p.BaseMyServiceClient.c}}).
> {code}
> func (p *MyServiceClient) DoStuff(ctx context.Context, request 
> *DoStuffRequest) (r *DoStuffResponse, err error) {
>   var _args139 DoStuffArgs
>   _args139.Request = request
>   var _result140 DoStuffResult
>   if err = p.c.Call(ctx, "do_stuff", &_args139, &_result140); err != nil 
> { // PANIC
>   ...
> {code}
> In progress fix here :https://github.com/apache/thrift/pull/1461. The fix in 
> this PR merely sets both instances of {{TClient}} (which is what happens in 
> the non-deprecated {{New*Client}} function).
> This patch currently fails {{make -k check}} however, since 
> {{src/tutorial/tutorial.go}} tries to access a different package's version of 
> the BaseClient.
> {code}
> src/tutorial/tutorial.go:477:33: bc.c undefined (cannot refer to unexported 
> field or method c)
> {code}
> The fix for that test could possibly be to expose the BaseClient's instance 
> of {{c}} (by making it a capital and thus exported {{C}}), or adding an 
> accessor method {{C()}} or {{Client()}}.
> Possibly a better fix would be to either remove these deprecated methods, or 
> figure out which {{TClient}} is the correct one to set.
> I'm not sure which is preferred, so I'm hoping to get some feedback/input 
> here.



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

[GitHub] thrift issue #1461: THRIFT-4447: Golang: Fix panic on p.c.Call when using de...

[dcelasun]

Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1461
  
Any capitalized method on the client will potentially conflict with an RPC, 
e.g:

```thrift
service Foo {
string C()
}
```
will generate uncompilable code. I suggest we use something like `Client_` 
as the method name. It looks ugly, but there is precedent for it (conflicts 
with keywords etc).


---

[jira] [Commented] (THRIFT-4447) Golang: Panic on p.c.Call when using deprecated initializers

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4447:


Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1461
  
Any capitalized method on the client will potentially conflict with an RPC, 
e.g:

```thrift
service Foo {
string C()
}
```
will generate uncompilable code. I suggest we use something like `Client_` 
as the method name. It looks ugly, but there is precedent for it (conflicts 
with keywords etc).


> Golang: Panic on p.c.Call when using deprecated initializers
> 
>
> Key: THRIFT-4447
> URL: https://issues.apache.org/jira/browse/THRIFT-4447
> Project: Thrift
>  Issue Type: Bug
>  Components: Go - Compiler
>Affects Versions: 0.11.0
>Reporter: John Boiles
>
> Latest thrift:master can cause panics when using deprecated 
> `New*ClientFactory` and `New*ClientProtocol` functions. This happens because 
> both the Client and the BaseClient have an instance of a {{thrift.TClient}}. 
> The deprecated methods initialize the BaseClient's TClient, but other methods 
> use the Client's {{TClient}}.
> For example, current thrift master generates structs like this
> {code}
> type MyServiceClient struct {
>   c thrift.TClient
>   *MyServiceBaseClient
> }
> type MyServiceBaseClient struct {
>   c thrift.TClient
> }
> {code}
> And also a method like this:
> {code}
> func NewMyServiceClientFactory(t thrift.TTransport, f 
> thrift.TProtocolFactory) *MyServiceClient {
>   return {MyServiceBaseClient: 
> NewMyServiceBaseClientFactory(t, f)}
> }
> {code}
> If that method is used, later calls to service methods will panic, since 
> {{p.c}} is nil (the actual client was stored in {{p.BaseMyServiceClient.c}}).
> {code}
> func (p *MyServiceClient) DoStuff(ctx context.Context, request 
> *DoStuffRequest) (r *DoStuffResponse, err error) {
>   var _args139 DoStuffArgs
>   _args139.Request = request
>   var _result140 DoStuffResult
>   if err = p.c.Call(ctx, "do_stuff", &_args139, &_result140); err != nil 
> { // PANIC
>   ...
> {code}
> In progress fix here :https://github.com/apache/thrift/pull/1461. The fix in 
> this PR merely sets both instances of {{TClient}} (which is what happens in 
> the non-deprecated {{New*Client}} function).
> This patch currently fails {{make -k check}} however, since 
> {{src/tutorial/tutorial.go}} tries to access a different package's version of 
> the BaseClient.
> {code}
> src/tutorial/tutorial.go:477:33: bc.c undefined (cannot refer to unexported 
> field or method c)
> {code}
> The fix for that test could possibly be to expose the BaseClient's instance 
> of {{c}} (by making it a capital and thus exported {{C}}), or adding an 
> accessor method {{C()}} or {{Client()}}.
> Possibly a better fix would be to either remove these deprecated methods, or 
> figure out which {{TClient}} is the correct one to set.
> I'm not sure which is preferred, so I'm hoping to get some feedback/input 
> here.



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

[GitHub] thrift issue #1449: THRIFT-4434. Update .NET Core components, add tests for ...

[vgotra]

Github user vgotra commented on the issue:

https://github.com/apache/thrift/pull/1449
  
@jeking3 - updated header file to fix warnings, also reverted Autotools 
files to check builds


---

[jira] [Commented] (THRIFT-4434) Update .NET Core components, add tests for .Net Core library and .Net Core compiler, fix bugs and build process

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4434:


Github user vgotra commented on the issue:

https://github.com/apache/thrift/pull/1449
  
@jeking3 - updated header file to fix warnings, also reverted Autotools 
files to check builds


> Update .NET Core components, add tests for .Net Core library and .Net Core 
> compiler, fix bugs and build process
> ---
>
> Key: THRIFT-4434
> URL: https://issues.apache.org/jira/browse/THRIFT-4434
> Project: Thrift
>  Issue Type: Improvement
>  Components: .NETCore - Compiler, .NETCore - Library, Build Process
> Environment: Windows, Linux, MacOS
>Reporter: Volodymyr Gotra
>Assignee: Volodymyr Gotra
>Priority: Critical
>
> This pull request should:
> - highly improve the current version of .Net Core library and .Net Core 
> compiler and quality of code
> - improve and simplify build process
> - improve documentation related to .Net Core library and compiler
> - fix found bugs (some of bugs can be clarified like major - they are related 
> to porting of protocols from Java version and can be present in C# library)
> - add important unit tests for .Net Core library and .Net Core compiler
> - add possibility to easy add unit tests for compiler for other languages



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

[jira] [Comment Edited] (THRIFT-4413) Publish a Maven artifact for Thrift v0.11

[JIRA]

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

Buğra Gedik edited comment on THRIFT-4413 at 1/9/18 11:41 PM:
--

[~jking3]: Do you think the Thrift project has someone who can do this?


was (Author: bgedik):
[~jking3]: Do you think the Thrift project has someone who can actually do this?

> Publish a Maven artifact for Thrift v0.11
> -
>
> Key: THRIFT-4413
> URL: https://issues.apache.org/jira/browse/THRIFT-4413
> Project: Thrift
>  Issue Type: Task
>  Components: Java - Library
>Affects Versions: 0.11.0
>Reporter: Buğra Gedik
>Assignee: Jake Farrell
> Fix For: 0.11.0
>
>




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

[jira] [Commented] (THRIFT-4413) Publish a Maven artifact for Thrift v0.11

[JIRA]

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

Buğra Gedik commented on THRIFT-4413:
-

[~jking3]: Do you think the Thrift project has someone who can actually do this?

> Publish a Maven artifact for Thrift v0.11
> -
>
> Key: THRIFT-4413
> URL: https://issues.apache.org/jira/browse/THRIFT-4413
> Project: Thrift
>  Issue Type: Task
>  Components: Java - Library
>Affects Versions: 0.11.0
>Reporter: Buğra Gedik
>Assignee: Jake Farrell
> Fix For: 0.11.0
>
>




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

[jira] [Commented] (THRIFT-4447) Golang: Panic on p.c.Call when using deprecated initializers

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4447:


Github user johnboiles commented on the issue:

https://github.com/apache/thrift/pull/1461
  
This is a lot cleaner -- I like it. We still have the same issue with 
tests, though, if the base service is in a separate package -- you can't access 
the client as `.c` on the base service since it's lowercased.

Maybe we expose the client with a getter method? 

```
func (p *BaseClient) C() thrift.TClient {
return p.c
}
```


> Golang: Panic on p.c.Call when using deprecated initializers
> 
>
> Key: THRIFT-4447
> URL: https://issues.apache.org/jira/browse/THRIFT-4447
> Project: Thrift
>  Issue Type: Bug
>  Components: Go - Compiler
>Affects Versions: 0.11.0
>Reporter: John Boiles
>
> Latest thrift:master can cause panics when using deprecated 
> `New*ClientFactory` and `New*ClientProtocol` functions. This happens because 
> both the Client and the BaseClient have an instance of a {{thrift.TClient}}. 
> The deprecated methods initialize the BaseClient's TClient, but other methods 
> use the Client's {{TClient}}.
> For example, current thrift master generates structs like this
> {code}
> type MyServiceClient struct {
>   c thrift.TClient
>   *MyServiceBaseClient
> }
> type MyServiceBaseClient struct {
>   c thrift.TClient
> }
> {code}
> And also a method like this:
> {code}
> func NewMyServiceClientFactory(t thrift.TTransport, f 
> thrift.TProtocolFactory) *MyServiceClient {
>   return {MyServiceBaseClient: 
> NewMyServiceBaseClientFactory(t, f)}
> }
> {code}
> If that method is used, later calls to service methods will panic, since 
> {{p.c}} is nil (the actual client was stored in {{p.BaseMyServiceClient.c}}).
> {code}
> func (p *MyServiceClient) DoStuff(ctx context.Context, request 
> *DoStuffRequest) (r *DoStuffResponse, err error) {
>   var _args139 DoStuffArgs
>   _args139.Request = request
>   var _result140 DoStuffResult
>   if err = p.c.Call(ctx, "do_stuff", &_args139, &_result140); err != nil 
> { // PANIC
>   ...
> {code}
> In progress fix here :https://github.com/apache/thrift/pull/1461. The fix in 
> this PR merely sets both instances of {{TClient}} (which is what happens in 
> the non-deprecated {{New*Client}} function).
> This patch currently fails {{make -k check}} however, since 
> {{src/tutorial/tutorial.go}} tries to access a different package's version of 
> the BaseClient.
> {code}
> src/tutorial/tutorial.go:477:33: bc.c undefined (cannot refer to unexported 
> field or method c)
> {code}
> The fix for that test could possibly be to expose the BaseClient's instance 
> of {{c}} (by making it a capital and thus exported {{C}}), or adding an 
> accessor method {{C()}} or {{Client()}}.
> Possibly a better fix would be to either remove these deprecated methods, or 
> figure out which {{TClient}} is the correct one to set.
> I'm not sure which is preferred, so I'm hoping to get some feedback/input 
> here.



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

[GitHub] thrift issue #1461: THRIFT-4447: Golang: Fix panic on p.c.Call when using de...

[johnboiles]

Github user johnboiles commented on the issue:

https://github.com/apache/thrift/pull/1461
  
This is a lot cleaner -- I like it. We still have the same issue with 
tests, though, if the base service is in a separate package -- you can't access 
the client as `.c` on the base service since it's lowercased.

Maybe we expose the client with a getter method? 

```
func (p *BaseClient) C() thrift.TClient {
return p.c
}
```


---

[jira] [Commented] (THRIFT-4413) Publish a Maven artifact for Thrift v0.11

[Doug Cutting (JIRA)]

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

Doug Cutting commented on THRIFT-4413:
--

I am no longer involved in the Thrift project.

All committers should be able to deploy and publish to Apache's Nexus Maven 
repository.  If @jking is unable to get this to work perhaps he should file an 
issue in the Infra jira with category Nexus.  Ideally Thrift's full release 
process would be documented.  For example, Avro's release process, including 
publishing Maven artifacts, is documented at 
https://cwiki.apache.org/confluence/display/AVRO/How+To+Release.  Thrift's 
instructions do not seem to include publishing to Maven or other repositories. 
https://thrift.apache.org/docs/committers/HowToRelease


> Publish a Maven artifact for Thrift v0.11
> -
>
> Key: THRIFT-4413
> URL: https://issues.apache.org/jira/browse/THRIFT-4413
> Project: Thrift
>  Issue Type: Task
>  Components: Java - Library
>Affects Versions: 0.11.0
>Reporter: Buğra Gedik
>Assignee: Jake Farrell
> Fix For: 0.11.0
>
>




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

[jira] [Commented] (THRIFT-4434) Update .NET Core components, add tests for .Net Core library and .Net Core compiler, fix bugs and build process

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4434:


Github user vgotra commented on the issue:

https://github.com/apache/thrift/pull/1449
  
@jeking3 - It seems that CLang doesn't recognize 
"-Wno-error=maybe-uninitialized". Do you know how to add some switches for 
different compilers ? - Error mostly because of one struct in header file (but 
to normally fix it it will take some time - unit tests for IDL much more 
important - already fixed few important bugs because of unit tetst for .Net 
Core generator) - easier to add suppression correctly and then fix it correctly.


> Update .NET Core components, add tests for .Net Core library and .Net Core 
> compiler, fix bugs and build process
> ---
>
> Key: THRIFT-4434
> URL: https://issues.apache.org/jira/browse/THRIFT-4434
> Project: Thrift
>  Issue Type: Improvement
>  Components: .NETCore - Compiler, .NETCore - Library, Build Process
> Environment: Windows, Linux, MacOS
>Reporter: Volodymyr Gotra
>Assignee: Volodymyr Gotra
>Priority: Critical
>
> This pull request should:
> - highly improve the current version of .Net Core library and .Net Core 
> compiler and quality of code
> - improve and simplify build process
> - improve documentation related to .Net Core library and compiler
> - fix found bugs (some of bugs can be clarified like major - they are related 
> to porting of protocols from Java version and can be present in C# library)
> - add important unit tests for .Net Core library and .Net Core compiler
> - add possibility to easy add unit tests for compiler for other languages



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

[GitHub] thrift issue #1449: THRIFT-4434. Update .NET Core components, add tests for ...

[vgotra]

Github user vgotra commented on the issue:

https://github.com/apache/thrift/pull/1449
  
@jeking3 - It seems that CLang doesn't recognize 
"-Wno-error=maybe-uninitialized". Do you know how to add some switches for 
different compilers ? - Error mostly because of one struct in header file (but 
to normally fix it it will take some time - unit tests for IDL much more 
important - already fixed few important bugs because of unit tetst for .Net 
Core generator) - easier to add suppression correctly and then fix it correctly.


---

[jira] [Commented] (THRIFT-4447) Golang: Panic on p.c.Call when using deprecated initializers

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4447:


Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1461
  
> So basically the logic would be to only add `c: thrift.TClient` to the 
`*Client` struct if `extends.empty()` in the generator?

Exactly.


> Golang: Panic on p.c.Call when using deprecated initializers
> 
>
> Key: THRIFT-4447
> URL: https://issues.apache.org/jira/browse/THRIFT-4447
> Project: Thrift
>  Issue Type: Bug
>  Components: Go - Compiler
>Affects Versions: 0.11.0
>Reporter: John Boiles
>
> Latest thrift:master can cause panics when using deprecated 
> `New*ClientFactory` and `New*ClientProtocol` functions. This happens because 
> both the Client and the BaseClient have an instance of a {{thrift.TClient}}. 
> The deprecated methods initialize the BaseClient's TClient, but other methods 
> use the Client's {{TClient}}.
> For example, current thrift master generates structs like this
> {code}
> type MyServiceClient struct {
>   c thrift.TClient
>   *MyServiceBaseClient
> }
> type MyServiceBaseClient struct {
>   c thrift.TClient
> }
> {code}
> And also a method like this:
> {code}
> func NewMyServiceClientFactory(t thrift.TTransport, f 
> thrift.TProtocolFactory) *MyServiceClient {
>   return {MyServiceBaseClient: 
> NewMyServiceBaseClientFactory(t, f)}
> }
> {code}
> If that method is used, later calls to service methods will panic, since 
> {{p.c}} is nil (the actual client was stored in {{p.BaseMyServiceClient.c}}).
> {code}
> func (p *MyServiceClient) DoStuff(ctx context.Context, request 
> *DoStuffRequest) (r *DoStuffResponse, err error) {
>   var _args139 DoStuffArgs
>   _args139.Request = request
>   var _result140 DoStuffResult
>   if err = p.c.Call(ctx, "do_stuff", &_args139, &_result140); err != nil 
> { // PANIC
>   ...
> {code}
> In progress fix here :https://github.com/apache/thrift/pull/1461. The fix in 
> this PR merely sets both instances of {{TClient}} (which is what happens in 
> the non-deprecated {{New*Client}} function).
> This patch currently fails {{make -k check}} however, since 
> {{src/tutorial/tutorial.go}} tries to access a different package's version of 
> the BaseClient.
> {code}
> src/tutorial/tutorial.go:477:33: bc.c undefined (cannot refer to unexported 
> field or method c)
> {code}
> The fix for that test could possibly be to expose the BaseClient's instance 
> of {{c}} (by making it a capital and thus exported {{C}}), or adding an 
> accessor method {{C()}} or {{Client()}}.
> Possibly a better fix would be to either remove these deprecated methods, or 
> figure out which {{TClient}} is the correct one to set.
> I'm not sure which is preferred, so I'm hoping to get some feedback/input 
> here.



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

[GitHub] thrift issue #1461: THRIFT-4447: Golang: Fix panic on p.c.Call when using de...

[dcelasun]

Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1461
  
> So basically the logic would be to only add `c: thrift.TClient` to the 
`*Client` struct if `extends.empty()` in the generator?

Exactly.


---

[jira] [Commented] (THRIFT-4447) Golang: Panic on p.c.Call when using deprecated initializers

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4447:


Github user johnboiles commented on the issue:

https://github.com/apache/thrift/pull/1461
  
You're correct that it's a service extending another service; tbh I missed 
that too (I'm not the author of the `.thrift` files. I'm also generally new to 
thrift so still sorting out what should be generated).

So basically the logic would be to only add `c: thrift.TClient` to the 
`*Client` struct if `extends.empty()` in the generator? I'll take a look and 
see how hard that looks.


> Golang: Panic on p.c.Call when using deprecated initializers
> 
>
> Key: THRIFT-4447
> URL: https://issues.apache.org/jira/browse/THRIFT-4447
> Project: Thrift
>  Issue Type: Bug
>  Components: Go - Compiler
>Affects Versions: 0.11.0
>Reporter: John Boiles
>
> Latest thrift:master can cause panics when using deprecated 
> `New*ClientFactory` and `New*ClientProtocol` functions. This happens because 
> both the Client and the BaseClient have an instance of a {{thrift.TClient}}. 
> The deprecated methods initialize the BaseClient's TClient, but other methods 
> use the Client's {{TClient}}.
> For example, current thrift master generates structs like this
> {code}
> type MyServiceClient struct {
>   c thrift.TClient
>   *MyServiceBaseClient
> }
> type MyServiceBaseClient struct {
>   c thrift.TClient
> }
> {code}
> And also a method like this:
> {code}
> func NewMyServiceClientFactory(t thrift.TTransport, f 
> thrift.TProtocolFactory) *MyServiceClient {
>   return {MyServiceBaseClient: 
> NewMyServiceBaseClientFactory(t, f)}
> }
> {code}
> If that method is used, later calls to service methods will panic, since 
> {{p.c}} is nil (the actual client was stored in {{p.BaseMyServiceClient.c}}).
> {code}
> func (p *MyServiceClient) DoStuff(ctx context.Context, request 
> *DoStuffRequest) (r *DoStuffResponse, err error) {
>   var _args139 DoStuffArgs
>   _args139.Request = request
>   var _result140 DoStuffResult
>   if err = p.c.Call(ctx, "do_stuff", &_args139, &_result140); err != nil 
> { // PANIC
>   ...
> {code}
> In progress fix here :https://github.com/apache/thrift/pull/1461. The fix in 
> this PR merely sets both instances of {{TClient}} (which is what happens in 
> the non-deprecated {{New*Client}} function).
> This patch currently fails {{make -k check}} however, since 
> {{src/tutorial/tutorial.go}} tries to access a different package's version of 
> the BaseClient.
> {code}
> src/tutorial/tutorial.go:477:33: bc.c undefined (cannot refer to unexported 
> field or method c)
> {code}
> The fix for that test could possibly be to expose the BaseClient's instance 
> of {{c}} (by making it a capital and thus exported {{C}}), or adding an 
> accessor method {{C()}} or {{Client()}}.
> Possibly a better fix would be to either remove these deprecated methods, or 
> figure out which {{TClient}} is the correct one to set.
> I'm not sure which is preferred, so I'm hoping to get some feedback/input 
> here.



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

[GitHub] thrift issue #1461: THRIFT-4447: Golang: Fix panic on p.c.Call when using de...

[johnboiles]

Github user johnboiles commented on the issue:

https://github.com/apache/thrift/pull/1461
  
You're correct that it's a service extending another service; tbh I missed 
that too (I'm not the author of the `.thrift` files. I'm also generally new to 
thrift so still sorting out what should be generated).

So basically the logic would be to only add `c: thrift.TClient` to the 
`*Client` struct if `extends.empty()` in the generator? I'll take a look and 
see how hard that looks.


---

[jira] [Commented] (THRIFT-4434) Update .NET Core components, add tests for .Net Core library and .Net Core compiler, fix bugs and build process

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4434:


Github user vgotra commented on the issue:

https://github.com/apache/thrift/pull/1449
  
Updated tests, fixed some problems (added framed correct support at server 
side, updated tests and test known failures, etc.), reverted one line in 
netcore*.h file (instead added suppression of one warning to automake files). 

It seems, that at Ubuntu 17.10, it can build and run all tests for .net 
core (for server side and client side) for all pre-configured languages with 
automake (make cross).


> Update .NET Core components, add tests for .Net Core library and .Net Core 
> compiler, fix bugs and build process
> ---
>
> Key: THRIFT-4434
> URL: https://issues.apache.org/jira/browse/THRIFT-4434
> Project: Thrift
>  Issue Type: Improvement
>  Components: .NETCore - Compiler, .NETCore - Library, Build Process
> Environment: Windows, Linux, MacOS
>Reporter: Volodymyr Gotra
>Assignee: Volodymyr Gotra
>Priority: Critical
>
> This pull request should:
> - highly improve the current version of .Net Core library and .Net Core 
> compiler and quality of code
> - improve and simplify build process
> - improve documentation related to .Net Core library and compiler
> - fix found bugs (some of bugs can be clarified like major - they are related 
> to porting of protocols from Java version and can be present in C# library)
> - add important unit tests for .Net Core library and .Net Core compiler
> - add possibility to easy add unit tests for compiler for other languages



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

[GitHub] thrift issue #1449: THRIFT-4434. Update .NET Core components, add tests for ...

[vgotra]

Github user vgotra commented on the issue:

https://github.com/apache/thrift/pull/1449
  
Updated tests, fixed some problems (added framed correct support at server 
side, updated tests and test known failures, etc.), reverted one line in 
netcore*.h file (instead added suppression of one warning to automake files). 

It seems, that at Ubuntu 17.10, it can build and run all tests for .net 
core (for server side and client side) for all pre-configured languages with 
automake (make cross).


---

[jira] [Commented] (THRIFT-4447) Golang: Panic on p.c.Call when using deprecated initializers

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4447:


Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1461
  
Ah, you have a service extending another service. I missed that. In that 
case, I think removing `c thrift.TClient` from `UserServiceClient` (and 
constructor functions) is the cleanest option. That way, we would only have a 
`TClient` within the `UserServiceBase` and it can be reused by any extending 
service. Thoughts?


> Golang: Panic on p.c.Call when using deprecated initializers
> 
>
> Key: THRIFT-4447
> URL: https://issues.apache.org/jira/browse/THRIFT-4447
> Project: Thrift
>  Issue Type: Bug
>  Components: Go - Compiler
>Affects Versions: 0.11.0
>Reporter: John Boiles
>
> Latest thrift:master can cause panics when using deprecated 
> `New*ClientFactory` and `New*ClientProtocol` functions. This happens because 
> both the Client and the BaseClient have an instance of a {{thrift.TClient}}. 
> The deprecated methods initialize the BaseClient's TClient, but other methods 
> use the Client's {{TClient}}.
> For example, current thrift master generates structs like this
> {code}
> type MyServiceClient struct {
>   c thrift.TClient
>   *MyServiceBaseClient
> }
> type MyServiceBaseClient struct {
>   c thrift.TClient
> }
> {code}
> And also a method like this:
> {code}
> func NewMyServiceClientFactory(t thrift.TTransport, f 
> thrift.TProtocolFactory) *MyServiceClient {
>   return {MyServiceBaseClient: 
> NewMyServiceBaseClientFactory(t, f)}
> }
> {code}
> If that method is used, later calls to service methods will panic, since 
> {{p.c}} is nil (the actual client was stored in {{p.BaseMyServiceClient.c}}).
> {code}
> func (p *MyServiceClient) DoStuff(ctx context.Context, request 
> *DoStuffRequest) (r *DoStuffResponse, err error) {
>   var _args139 DoStuffArgs
>   _args139.Request = request
>   var _result140 DoStuffResult
>   if err = p.c.Call(ctx, "do_stuff", &_args139, &_result140); err != nil 
> { // PANIC
>   ...
> {code}
> In progress fix here :https://github.com/apache/thrift/pull/1461. The fix in 
> this PR merely sets both instances of {{TClient}} (which is what happens in 
> the non-deprecated {{New*Client}} function).
> This patch currently fails {{make -k check}} however, since 
> {{src/tutorial/tutorial.go}} tries to access a different package's version of 
> the BaseClient.
> {code}
> src/tutorial/tutorial.go:477:33: bc.c undefined (cannot refer to unexported 
> field or method c)
> {code}
> The fix for that test could possibly be to expose the BaseClient's instance 
> of {{c}} (by making it a capital and thus exported {{C}}), or adding an 
> accessor method {{C()}} or {{Client()}}.
> Possibly a better fix would be to either remove these deprecated methods, or 
> figure out which {{TClient}} is the correct one to set.
> I'm not sure which is preferred, so I'm hoping to get some feedback/input 
> here.



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

[GitHub] thrift issue #1461: THRIFT-4447: Golang: Fix panic on p.c.Call when using de...

[dcelasun]

Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1461
  
Ah, you have a service extending another service. I missed that. In that 
case, I think removing `c thrift.TClient` from `UserServiceClient` (and 
constructor functions) is the cleanest option. That way, we would only have a 
`TClient` within the `UserServiceBase` and it can be reused by any extending 
service. Thoughts?


---

[jira] [Commented] (THRIFT-4447) Golang: Panic on p.c.Call when using deprecated initializers

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4447:


Github user johnboiles commented on the issue:

https://github.com/apache/thrift/pull/1461
  
> Are you getting this second "Base" struct with 0.11 compiler? Because I 
can't reproduce.

I've been working off the master branch, but I just tried generation with 
0.11 (327ebb6c2b6df8bf075da02ef45a2a034e9b79ba) and it looks like it has the 
same issue. Here is a direct copy-paste from my generated code with 0.11:

```
// Deprecated: Use NewUserService instead
func NewUserServiceClientFactory(t thrift.TTransport, f 
thrift.TProtocolFactory) *UserServiceClient {
return {UserServiceBaseClient: 
NewUserServiceBaseClientFactory(t, f)}
}

// Deprecated: Use NewUserService instead
func NewUserServiceClientProtocol(t thrift.TTransport, iprot 
thrift.TProtocol, oprot thrift.TProtocol) *UserServiceClient {
return {UserServiceBaseClient: 
NewUserServiceBaseClientProtocol(t, iprot, oprot)}
}

func NewUserServiceClient(c thrift.TClient) *UserServiceClient {
return {
c: c,
UserServiceBaseClient: NewUserServiceBaseClient(c),
}
}
```


> Golang: Panic on p.c.Call when using deprecated initializers
> 
>
> Key: THRIFT-4447
> URL: https://issues.apache.org/jira/browse/THRIFT-4447
> Project: Thrift
>  Issue Type: Bug
>  Components: Go - Compiler
>Affects Versions: 0.11.0
>Reporter: John Boiles
>
> Latest thrift:master can cause panics when using deprecated 
> `New*ClientFactory` and `New*ClientProtocol` functions. This happens because 
> both the Client and the BaseClient have an instance of a {{thrift.TClient}}. 
> The deprecated methods initialize the BaseClient's TClient, but other methods 
> use the Client's {{TClient}}.
> For example, current thrift master generates structs like this
> {code}
> type MyServiceClient struct {
>   c thrift.TClient
>   *MyServiceBaseClient
> }
> type MyServiceBaseClient struct {
>   c thrift.TClient
> }
> {code}
> And also a method like this:
> {code}
> func NewMyServiceClientFactory(t thrift.TTransport, f 
> thrift.TProtocolFactory) *MyServiceClient {
>   return {MyServiceBaseClient: 
> NewMyServiceBaseClientFactory(t, f)}
> }
> {code}
> If that method is used, later calls to service methods will panic, since 
> {{p.c}} is nil (the actual client was stored in {{p.BaseMyServiceClient.c}}).
> {code}
> func (p *MyServiceClient) DoStuff(ctx context.Context, request 
> *DoStuffRequest) (r *DoStuffResponse, err error) {
>   var _args139 DoStuffArgs
>   _args139.Request = request
>   var _result140 DoStuffResult
>   if err = p.c.Call(ctx, "do_stuff", &_args139, &_result140); err != nil 
> { // PANIC
>   ...
> {code}
> In progress fix here :https://github.com/apache/thrift/pull/1461. The fix in 
> this PR merely sets both instances of {{TClient}} (which is what happens in 
> the non-deprecated {{New*Client}} function).
> This patch currently fails {{make -k check}} however, since 
> {{src/tutorial/tutorial.go}} tries to access a different package's version of 
> the BaseClient.
> {code}
> src/tutorial/tutorial.go:477:33: bc.c undefined (cannot refer to unexported 
> field or method c)
> {code}
> The fix for that test could possibly be to expose the BaseClient's instance 
> of {{c}} (by making it a capital and thus exported {{C}}), or adding an 
> accessor method {{C()}} or {{Client()}}.
> Possibly a better fix would be to either remove these deprecated methods, or 
> figure out which {{TClient}} is the correct one to set.
> I'm not sure which is preferred, so I'm hoping to get some feedback/input 
> here.



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

[GitHub] thrift issue #1461: THRIFT-4447: Golang: Fix panic on p.c.Call when using de...

[johnboiles]

Github user johnboiles commented on the issue:

https://github.com/apache/thrift/pull/1461
  
> Are you getting this second "Base" struct with 0.11 compiler? Because I 
can't reproduce.

I've been working off the master branch, but I just tried generation with 
0.11 (327ebb6c2b6df8bf075da02ef45a2a034e9b79ba) and it looks like it has the 
same issue. Here is a direct copy-paste from my generated code with 0.11:

```
// Deprecated: Use NewUserService instead
func NewUserServiceClientFactory(t thrift.TTransport, f 
thrift.TProtocolFactory) *UserServiceClient {
return {UserServiceBaseClient: 
NewUserServiceBaseClientFactory(t, f)}
}

// Deprecated: Use NewUserService instead
func NewUserServiceClientProtocol(t thrift.TTransport, iprot 
thrift.TProtocol, oprot thrift.TProtocol) *UserServiceClient {
return {UserServiceBaseClient: 
NewUserServiceBaseClientProtocol(t, iprot, oprot)}
}

func NewUserServiceClient(c thrift.TClient) *UserServiceClient {
return {
c: c,
UserServiceBaseClient: NewUserServiceBaseClient(c),
}
}
```


---

[jira] [Comment Edited] (THRIFT-4413) Publish a Maven artifact for Thrift v0.11

[JIRA]

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

Buğra Gedik edited comment on THRIFT-4413 at 1/9/18 8:19 PM:
-

There are no comments from [~jfarrell] or anyone else and this is pretty much 
stuck. Does anyone know what action can be taken? 

[~cutting]: I saw your name in this page: https://thrift.apache.org/about as 
the project champion and as such, I am mentioning you here. We are unable to 
get Thrift 0.11.0 into Maven and PyPi, despite reporting the issue here and in 
THRIFT-4414. It seems the people who have the authority to do this are not 
responding to our requests. The same ignorance was shown here: 
https://issues.apache.org/jira/browse/THRIFT-4250. If you read that thread, you 
can see how the release was handled improperly ([~ctubbsii]'s comment 
[here|https://issues.apache.org/jira/browse/THRIFT-4250?focusedCommentId=16291576=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16291576]
 summarizes it really well). 


was (Author: bgedik):
There are no comments from [~jfarrell] or anyone else and this is pretty much 
stuck. Does anyone know what action can be taken? 

[~cutting]: I saw your name in this page: https://thrift.apache.org/about as 
the project champion and as such, I am mentioning you here. We are unable to 
get Thrift 0.11.0 in Maven and PyPi, despite reporting the issue here and in 
THRIFT-4414. It seems the people who have the authority to do this are not 
responding to our requests. The same ignorance was shown here: 
https://issues.apache.org/jira/browse/THRIFT-4250. If you read that thread, you 
can see how the release was handled improperly ([~ctubbsii]'s comment 
[here|https://issues.apache.org/jira/browse/THRIFT-4250?focusedCommentId=16291576=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16291576]
 summarizes it really well). 

> Publish a Maven artifact for Thrift v0.11
> -
>
> Key: THRIFT-4413
> URL: https://issues.apache.org/jira/browse/THRIFT-4413
> Project: Thrift
>  Issue Type: Task
>  Components: Java - Library
>Affects Versions: 0.11.0
>Reporter: Buğra Gedik
>Assignee: Jake Farrell
> Fix For: 0.11.0
>
>




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

[jira] [Comment Edited] (THRIFT-4413) Publish a Maven artifact for Thrift v0.11

[JIRA]

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

Buğra Gedik edited comment on THRIFT-4413 at 1/9/18 8:17 PM:
-

There are no comments from [~jfarrell] or anyone else and this is pretty much 
stuck. Does anyone know what action can be taken? 

[~cutting]: I saw your name in this page: https://thrift.apache.org/about as 
the project champion and as such, I am mentioning you here. We are unable to 
get Thrift 0.11.0 in Maven and PyPi, despite reporting the issue here and in 
THRIFT-4414. It seems the people who have the authority to do this are not 
responding to our requests. The same ignorance was shown here: 
https://issues.apache.org/jira/browse/THRIFT-4250. If you read that thread, you 
can see how the release was handled improperly ([~ctubbsii]'s comment 
[here|https://issues.apache.org/jira/browse/THRIFT-4250?focusedCommentId=16291576=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16291576]
 summarizes it really well). 


was (Author: bgedik):
There are no comments from [~jfarrell] or anyone else and this is pretty much 
stuck. Does anyone know what action can be taken? 

[~cutting]: I saw your name in this page: https://thrift.apache.org/about as 
the project champion and as such, mentioning you here. We are unable to get 
Thrift 0.11.0 in maven and PyPi, despite reporting the issue here and in 
THRIFT-4414. It seems the people who have the authority to do this are not 
responding to our requests. The same ignorance was shown here: 
https://issues.apache.org/jira/browse/THRIFT-4250. If you read that thread, you 
can see how the release was handled improperly ([~ctubbsii]'s comment 
[here|https://issues.apache.org/jira/browse/THRIFT-4250?focusedCommentId=16291576=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16291576]
 summarizes it really well). 

> Publish a Maven artifact for Thrift v0.11
> -
>
> Key: THRIFT-4413
> URL: https://issues.apache.org/jira/browse/THRIFT-4413
> Project: Thrift
>  Issue Type: Task
>  Components: Java - Library
>Affects Versions: 0.11.0
>Reporter: Buğra Gedik
>Assignee: Jake Farrell
> Fix For: 0.11.0
>
>




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

[jira] [Comment Edited] (THRIFT-4413) Publish a Maven artifact for Thrift v0.11

[JIRA]

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

Buğra Gedik edited comment on THRIFT-4413 at 1/9/18 8:16 PM:
-

There are no comments from [~jfarrell] or anyone else and this is pretty much 
stuck. Does anyone know what action can be taken? 

[~cutting]: I saw your name in this page: https://thrift.apache.org/about as 
the project champion and as such, mentioning you here. We are unable to get 
Thrift 0.11.0 in maven and PyPi, despite reporting the issue here and in 
THRIFT-4414. It seems the people who have the authority to do this are not 
responding to our requests. The same ignorance was shown here: 
https://issues.apache.org/jira/browse/THRIFT-4250. If you read that thread, you 
can see how the release was handled improperly ([~ctubbsii]'s comment 
[here|https://issues.apache.org/jira/browse/THRIFT-4250?focusedCommentId=16291576=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16291576]
 summarizes it really well). 


was (Author: bgedik):
There are no comment from [~jfarrell] or anyone else and this is pretty much 
stuck. Does anyone know what action can be taken? 

[~cutting]: I saw your name in this page: https://thrift.apache.org/about as 
the project champion and as such, mentioning you here. We are unable to get 
Thrift 0.11.0 in maven and PyPi, despite reporting the issue here and in 
THRIFT-4414. It seems the people who have the authority to do this are not 
responding to our requests. The same ignorance was shown here: 
https://issues.apache.org/jira/browse/THRIFT-4250. If you read that thread, you 
can see how the release was handled improperly ([~ctubbsii]'s comment 
[here|https://issues.apache.org/jira/browse/THRIFT-4250?focusedCommentId=16291576=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16291576]
 summarizes it really well). 

> Publish a Maven artifact for Thrift v0.11
> -
>
> Key: THRIFT-4413
> URL: https://issues.apache.org/jira/browse/THRIFT-4413
> Project: Thrift
>  Issue Type: Task
>  Components: Java - Library
>Affects Versions: 0.11.0
>Reporter: Buğra Gedik
>Assignee: Jake Farrell
> Fix For: 0.11.0
>
>




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

[jira] [Comment Edited] (THRIFT-4413) Publish a Maven artifact for Thrift v0.11

[JIRA]

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

Buğra Gedik edited comment on THRIFT-4413 at 1/9/18 8:16 PM:
-

There are no comment from [~jfarrell] or anyone else and this is pretty much 
stuck. Does anyone know what action can be taken? 

[~cutting]: I saw your name in this page: https://thrift.apache.org/about as 
the project champion and as such, mentioning you here. We are unable to get 
Thrift 0.11.0 in maven and PyPi, despite reporting the issue here and in 
THRIFT-4414. It seems the people who have the authority to do this are not 
responding to our requests. The same ignorance was shown here: 
https://issues.apache.org/jira/browse/THRIFT-4250. If you read that thread, you 
can see how the release was handled improperly ([~ctubbsii]'s comment 
[here|https://issues.apache.org/jira/browse/THRIFT-4250?focusedCommentId=16291576=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16291576]
 summarizes it really well). 


was (Author: bgedik):
There no comment from [~jfarrell] or anyone else and this is pretty much stuck. 
Does anyone know what action can be taken? 

[~cutting]: I saw your name in this page: https://thrift.apache.org/about as 
the project champion and as such, mentioning you here. We are unable to get 
Thrift 0.11.0 in maven and PyPi, despite reporting the issue here and in 
THRIFT-4414. It seems the people who have the authority to do this are not 
responding to our requests. The same ignorance was shown here: 
https://issues.apache.org/jira/browse/THRIFT-4250. If you read that thread, you 
can see how the release was handled improperly ([~ctubbsii]'s comment 
[here|https://issues.apache.org/jira/browse/THRIFT-4250?focusedCommentId=16291576=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16291576]
 summarizes it really well). 

> Publish a Maven artifact for Thrift v0.11
> -
>
> Key: THRIFT-4413
> URL: https://issues.apache.org/jira/browse/THRIFT-4413
> Project: Thrift
>  Issue Type: Task
>  Components: Java - Library
>Affects Versions: 0.11.0
>Reporter: Buğra Gedik
>Assignee: Jake Farrell
> Fix For: 0.11.0
>
>




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

[jira] [Comment Edited] (THRIFT-4413) Publish a Maven artifact for Thrift v0.11

[JIRA]

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

Buğra Gedik edited comment on THRIFT-4413 at 1/9/18 8:16 PM:
-

There no comment from [~jfarrell] or anyone else and this is pretty much stuck. 
Does anyone know what action can be taken? 

[~cutting]: I saw your name in this page: https://thrift.apache.org/about as 
the project champion and as such, mentioning you here. We are unable to get 
Thrift 0.11.0 in maven and PyPi, despite reporting the issue here and in 
THRIFT-4414. It seems the people who have the authority to do this are not 
responding to our requests. The same ignorance was shown here: 
https://issues.apache.org/jira/browse/THRIFT-4250. If you read that thread, you 
can see how the release was handled improperly ([~ctubbsii]'s comment 
[here|https://issues.apache.org/jira/browse/THRIFT-4250?focusedCommentId=16291576=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16291576]
 summarizes it really well). 


was (Author: bgedik):
There no comment from [~jfarrell] or anyone else and this is pretty much stuck. 
Does anyone know what action can be taken? 

[~cutting]: I saw your name in this page: https://thrift.apache.org/about as 
the project champion and as such, mentioning you here. We are unable to get 
Thrift 0.11.0 in maven and PyPi, despite reporting the issue here and in 
THRIFT-4414. It seems the people who have the authority to do this are not 
responding to our requests. The same ignorance was shown here: 
https://issues.apache.org/jira/browse/THRIFT-4250. If you read that thread, you 
can see how the release was handled improperly ([~ctubbsii]'s comment 
[https://issues.apache.org/jira/browse/THRIFT-4250?focusedCommentId=16291576=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16291576|here]
 summarizes it really well). 

> Publish a Maven artifact for Thrift v0.11
> -
>
> Key: THRIFT-4413
> URL: https://issues.apache.org/jira/browse/THRIFT-4413
> Project: Thrift
>  Issue Type: Task
>  Components: Java - Library
>Affects Versions: 0.11.0
>Reporter: Buğra Gedik
>Assignee: Jake Farrell
> Fix For: 0.11.0
>
>




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

[jira] [Commented] (THRIFT-4413) Publish a Maven artifact for Thrift v0.11

[JIRA]

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

Buğra Gedik commented on THRIFT-4413:
-

There no comment from [~jfarrell] or anyone else and this is pretty much stuck. 
Does anyone know what action can be taken? 

[~cutting]: I saw your name in this page: https://thrift.apache.org/about as 
the project champion and as such, mentioning you here. We are unable to get 
Thrift 0.11.0 in maven and PyPi, despite reporting the issue here and in 
THRIFT-4414. It seems the people who have the authority to do this are not 
responding to our requests. The same ignorance was shown here: 
https://issues.apache.org/jira/browse/THRIFT-4250. If you read that thread, you 
can see how the release was handled improperly ([~ctubbsii]'s comment 
[https://issues.apache.org/jira/browse/THRIFT-4250?focusedCommentId=16291576=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16291576|here]
 summarizes it really well). 

> Publish a Maven artifact for Thrift v0.11
> -
>
> Key: THRIFT-4413
> URL: https://issues.apache.org/jira/browse/THRIFT-4413
> Project: Thrift
>  Issue Type: Task
>  Components: Java - Library
>Affects Versions: 0.11.0
>Reporter: Buğra Gedik
>Assignee: Jake Farrell
> Fix For: 0.11.0
>
>




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

[jira] [Commented] (THRIFT-4448) Golang: do something with context.Context

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4448:


Github user johnboiles commented on the issue:

https://github.com/apache/thrift/pull/1459
  
Thank you for the explainer @dcelasun! Makes sense to me.

@Jens-G, @jeking3: if one of you sign off on this, I'll make the changes to 
drop go1.6 and `x/net/context` support from thrift:master.


> Golang: do something with context.Context
> -
>
> Key: THRIFT-4448
> URL: https://issues.apache.org/jira/browse/THRIFT-4448
> Project: Thrift
>  Issue Type: Task
>  Components: Go - Library
>Affects Versions: 0.11.0
>Reporter: John Boiles
>
> PR Here: https://github.com/apache/thrift/pull/1459
> This patch wires through {{context.Context}} such that it can be used in in 
> {{http.Request}}'s {{WithContext}} method. This allows Thrift HTTP requests 
> to canceled or timed out via the context.
> This patch breaks support for go<1.7 so it's not ready to ship, but I'm 
> hoping to get some direction on this. When does Thrift expect to drop support 
> of go1.7? It looks like the current solution is to duplicate files that need 
> to use {{golang.org/x/net/context}} and add a {{// +build !go1.7}} but 
> duplication seems unsustainable as the {{context}} package is imported more 
> places.
> Go 1.7 was released 15 August 2016. Given Golang has had significant 
> performance improvements in most dot releases, I suspect most production 
> services stay reasonably up to date. Here at Periscope/Twitter we're on 
> go1.9.1, and we're a fairly large organization.



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

[GitHub] thrift issue #1459: THRIFT-4448: Golang: do something with context.Context

[johnboiles]

Github user johnboiles commented on the issue:

https://github.com/apache/thrift/pull/1459
  
Thank you for the explainer @dcelasun! Makes sense to me.

@Jens-G, @jeking3: if one of you sign off on this, I'll make the changes to 
drop go1.6 and `x/net/context` support from thrift:master.


---

[jira] [Commented] (THRIFT-4448) Golang: do something with context.Context

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4448:


Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1459
  
> I personally think dropping go1.6/x/net/context support is best for the 
project. Are you the right person to make that call?

Sounds good to me, but we need to run it by someone with commit bits. cc 
@Jens-G @jeking3 

> Related question: why was 'context support' added when it doesn't seem to 
be hooked up to anything in the library? Seems like the method signatures were 
the only thing that changed.

It was added to support passing arbitrary values into RPC handlers. One of 
the big use cases was accessing client IP from an RPC handler. Before 
`context`, this was impossible since the underlying transport was not visible 
to the handler. Now, you can implement a custom `TProcessor` that embeds the 
generated processor, and pass the IP manually, something like:

```go
type MyProcessor struct {
*GeneratedProcessor
}

func (p *MyProcessor) Process(ctx context.Context, in, out TProtocol) 
(bool, TException) {
name, _, seqId, err := iprot.ReadMessageBegin()
if err != nil {
return false, err
}
if processor, ok := p.GetProcessorFunction(name); ok {
//TODO: get IP from transport and add to ctx

return processor.Process(ctx, seqId, iprot, oprot)
}

// rest of it...
}
}

```


> Golang: do something with context.Context
> -
>
> Key: THRIFT-4448
> URL: https://issues.apache.org/jira/browse/THRIFT-4448
> Project: Thrift
>  Issue Type: Task
>  Components: Go - Library
>Affects Versions: 0.11.0
>Reporter: John Boiles
>
> PR Here: https://github.com/apache/thrift/pull/1459
> This patch wires through {{context.Context}} such that it can be used in in 
> {{http.Request}}'s {{WithContext}} method. This allows Thrift HTTP requests 
> to canceled or timed out via the context.
> This patch breaks support for go<1.7 so it's not ready to ship, but I'm 
> hoping to get some direction on this. When does Thrift expect to drop support 
> of go1.7? It looks like the current solution is to duplicate files that need 
> to use {{golang.org/x/net/context}} and add a {{// +build !go1.7}} but 
> duplication seems unsustainable as the {{context}} package is imported more 
> places.
> Go 1.7 was released 15 August 2016. Given Golang has had significant 
> performance improvements in most dot releases, I suspect most production 
> services stay reasonably up to date. Here at Periscope/Twitter we're on 
> go1.9.1, and we're a fairly large organization.



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

[GitHub] thrift issue #1459: THRIFT-4448: Golang: do something with context.Context

[dcelasun]

Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1459
  
> I personally think dropping go1.6/x/net/context support is best for the 
project. Are you the right person to make that call?

Sounds good to me, but we need to run it by someone with commit bits. cc 
@Jens-G @jeking3 

> Related question: why was 'context support' added when it doesn't seem to 
be hooked up to anything in the library? Seems like the method signatures were 
the only thing that changed.

It was added to support passing arbitrary values into RPC handlers. One of 
the big use cases was accessing client IP from an RPC handler. Before 
`context`, this was impossible since the underlying transport was not visible 
to the handler. Now, you can implement a custom `TProcessor` that embeds the 
generated processor, and pass the IP manually, something like:

```go
type MyProcessor struct {
*GeneratedProcessor
}

func (p *MyProcessor) Process(ctx context.Context, in, out TProtocol) 
(bool, TException) {
name, _, seqId, err := iprot.ReadMessageBegin()
if err != nil {
return false, err
}
if processor, ok := p.GetProcessorFunction(name); ok {
//TODO: get IP from transport and add to ctx

return processor.Process(ctx, seqId, iprot, oprot)
}

// rest of it...
}
}

```


---

[GitHub] thrift issue #1459: THRIFT-4448: Golang: do something with context.Context

[johnboiles]

Github user johnboiles commented on the issue:

https://github.com/apache/thrift/pull/1459
  
I really appreciate your feedback & involvement @dcelasun! I personally 
think dropping `go1.6`/`x/net/context` support is best for the project. Are you 
the right person to make that call? Or are there others we should check with 
before I put in the work.

Related question: why _was_ 'context support' added when it doesn't seem to 
be hooked up to anything in the library? Seems like the method signatures were 
the only thing that changed.


---

[jira] [Commented] (THRIFT-4448) Golang: do something with context.Context

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4448:


Github user johnboiles commented on the issue:

https://github.com/apache/thrift/pull/1459
  
I really appreciate your feedback & involvement @dcelasun! I personally 
think dropping `go1.6`/`x/net/context` support is best for the project. Are you 
the right person to make that call? Or are there others we should check with 
before I put in the work.

Related question: why _was_ 'context support' added when it doesn't seem to 
be hooked up to anything in the library? Seems like the method signatures were 
the only thing that changed.


> Golang: do something with context.Context
> -
>
> Key: THRIFT-4448
> URL: https://issues.apache.org/jira/browse/THRIFT-4448
> Project: Thrift
>  Issue Type: Task
>  Components: Go - Library
>Affects Versions: 0.11.0
>Reporter: John Boiles
>
> PR Here: https://github.com/apache/thrift/pull/1459
> This patch wires through {{context.Context}} such that it can be used in in 
> {{http.Request}}'s {{WithContext}} method. This allows Thrift HTTP requests 
> to canceled or timed out via the context.
> This patch breaks support for go<1.7 so it's not ready to ship, but I'm 
> hoping to get some direction on this. When does Thrift expect to drop support 
> of go1.7? It looks like the current solution is to duplicate files that need 
> to use {{golang.org/x/net/context}} and add a {{// +build !go1.7}} but 
> duplication seems unsustainable as the {{context}} package is imported more 
> places.
> Go 1.7 was released 15 August 2016. Given Golang has had significant 
> performance improvements in most dot releases, I suspect most production 
> services stay reasonably up to date. Here at Periscope/Twitter we're on 
> go1.9.1, and we're a fairly large organization.



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

[jira] [Commented] (THRIFT-4390) Rust binary protocol and buffered transport cannot handle writes above 4096 bytes

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4390:


Github user allengeorge commented on the issue:

https://github.com/apache/thrift/pull/1458
  
@jeking3 This seems like a separate problem unfortunately. I'd removed 
three tests from the "known failures list" and it appears there's a specific 
problem related to multiplexed processors and c_glib/perl. That was an 
oversight :/

I'd suggest the following course of action:

1. I re-add those three test failures to the "known failures" list
2. I fix up the framed test failures with > 4k long messages; I have a 
patch for this already
3. I add a JIRA for the multiplexed failures and fix that in a separate PR

Let me know if this is acceptable to you, and I'll go about doing that.


> Rust binary protocol and buffered transport cannot handle writes above 4096 
> bytes
> -
>
> Key: THRIFT-4390
> URL: https://issues.apache.org/jira/browse/THRIFT-4390
> Project: Thrift
>  Issue Type: Bug
>  Components: Rust - Library
>Affects Versions: 0.10.0
> Environment: docker image ubuntu-artful
>Reporter: James E. King, III
>Assignee: Allen George
>Priority: Critical
>
> While working on improving test coverage and fixing busted cross tests I 
> reworked the cpp test client to send binary in at size 0, 1, 2, 4, 6, 16, 
> ..., 131072 and after 4096 the rust server gave up.
> {noformat}
> 12, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126, 
> 127, 128])
> WARN:thrift::server::threaded: processor completed with error: TransportError 
> { kind: Unknown, message: "failed to write whole buffer" }
> Server process is successfully killed.
> {noformat}
> @gadLinux this may be the root cause of some of the issues you were seeing 
> with the interop against c_glib recently.  It is the root cause of some (if 
> not all of) the rs-csharp test failures.



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

[GitHub] thrift issue #1458: THRIFT-4390: Fix bug where binary/buffered messages > 4K...

[allengeorge]

Github user allengeorge commented on the issue:

https://github.com/apache/thrift/pull/1458
  
@jeking3 This seems like a separate problem unfortunately. I'd removed 
three tests from the "known failures list" and it appears there's a specific 
problem related to multiplexed processors and c_glib/perl. That was an 
oversight :/

I'd suggest the following course of action:

1. I re-add those three test failures to the "known failures" list
2. I fix up the framed test failures with > 4k long messages; I have a 
patch for this already
3. I add a JIRA for the multiplexed failures and fix that in a separate PR

Let me know if this is acceptable to you, and I'll go about doing that.


---

[jira] [Commented] (THRIFT-4448) Golang: do something with context.Context

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4448:


Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1459
  
When context support was added, significant effort went into keeping 1.7 
compatibility so I think it would be great if we could maintain that support.

On the other hand, this patch can go into 0.12 at the earliest and by that 
time 1.7 will be ancient. I haven't reviewed the patch yet, but if you'd like 
to drop compatibility, please do so across the codebase (so no more 
`x/net/context` anywhere).


> Golang: do something with context.Context
> -
>
> Key: THRIFT-4448
> URL: https://issues.apache.org/jira/browse/THRIFT-4448
> Project: Thrift
>  Issue Type: Task
>  Components: Go - Library
>Affects Versions: 0.11.0
>Reporter: John Boiles
>
> PR Here: https://github.com/apache/thrift/pull/1459
> This patch wires through {{context.Context}} such that it can be used in in 
> {{http.Request}}'s {{WithContext}} method. This allows Thrift HTTP requests 
> to canceled or timed out via the context.
> This patch breaks support for go<1.7 so it's not ready to ship, but I'm 
> hoping to get some direction on this. When does Thrift expect to drop support 
> of go1.7? It looks like the current solution is to duplicate files that need 
> to use {{golang.org/x/net/context}} and add a {{// +build !go1.7}} but 
> duplication seems unsustainable as the {{context}} package is imported more 
> places.
> Go 1.7 was released 15 August 2016. Given Golang has had significant 
> performance improvements in most dot releases, I suspect most production 
> services stay reasonably up to date. Here at Periscope/Twitter we're on 
> go1.9.1, and we're a fairly large organization.



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

[GitHub] thrift issue #1459: THRIFT-4448: Golang: do something with context.Context

[dcelasun]

Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1459
  
When context support was added, significant effort went into keeping 1.7 
compatibility so I think it would be great if we could maintain that support.

On the other hand, this patch can go into 0.12 at the earliest and by that 
time 1.7 will be ancient. I haven't reviewed the patch yet, but if you'd like 
to drop compatibility, please do so across the codebase (so no more 
`x/net/context` anywhere).


---

[jira] [Commented] (THRIFT-4447) Golang: Panic on p.c.Call when using deprecated initializers

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4447:


Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1461
  
```go
type MyServiceClient struct {
c thrift.TClient
*MyServiceBaseClient
}

type MyServiceBaseClient struct {
c thrift.TClient
}
```

@johnboiles Are you getting this second "Base" struct with 0.11 compiler? 
Because I can't reproduce.

> Also should I remove the // Deprecated comments if these are not actually 
deprecated?

Yes please.


> Golang: Panic on p.c.Call when using deprecated initializers
> 
>
> Key: THRIFT-4447
> URL: https://issues.apache.org/jira/browse/THRIFT-4447
> Project: Thrift
>  Issue Type: Bug
>  Components: Go - Compiler
>Affects Versions: 0.11.0
>Reporter: John Boiles
>
> Latest thrift:master can cause panics when using deprecated 
> `New*ClientFactory` and `New*ClientProtocol` functions. This happens because 
> both the Client and the BaseClient have an instance of a {{thrift.TClient}}. 
> The deprecated methods initialize the BaseClient's TClient, but other methods 
> use the Client's {{TClient}}.
> For example, current thrift master generates structs like this
> {code}
> type MyServiceClient struct {
>   c thrift.TClient
>   *MyServiceBaseClient
> }
> type MyServiceBaseClient struct {
>   c thrift.TClient
> }
> {code}
> And also a method like this:
> {code}
> func NewMyServiceClientFactory(t thrift.TTransport, f 
> thrift.TProtocolFactory) *MyServiceClient {
>   return {MyServiceBaseClient: 
> NewMyServiceBaseClientFactory(t, f)}
> }
> {code}
> If that method is used, later calls to service methods will panic, since 
> {{p.c}} is nil (the actual client was stored in {{p.BaseMyServiceClient.c}}).
> {code}
> func (p *MyServiceClient) DoStuff(ctx context.Context, request 
> *DoStuffRequest) (r *DoStuffResponse, err error) {
>   var _args139 DoStuffArgs
>   _args139.Request = request
>   var _result140 DoStuffResult
>   if err = p.c.Call(ctx, "do_stuff", &_args139, &_result140); err != nil 
> { // PANIC
>   ...
> {code}
> In progress fix here :https://github.com/apache/thrift/pull/1461. The fix in 
> this PR merely sets both instances of {{TClient}} (which is what happens in 
> the non-deprecated {{New*Client}} function).
> This patch currently fails {{make -k check}} however, since 
> {{src/tutorial/tutorial.go}} tries to access a different package's version of 
> the BaseClient.
> {code}
> src/tutorial/tutorial.go:477:33: bc.c undefined (cannot refer to unexported 
> field or method c)
> {code}
> The fix for that test could possibly be to expose the BaseClient's instance 
> of {{c}} (by making it a capital and thus exported {{C}}), or adding an 
> accessor method {{C()}} or {{Client()}}.
> Possibly a better fix would be to either remove these deprecated methods, or 
> figure out which {{TClient}} is the correct one to set.
> I'm not sure which is preferred, so I'm hoping to get some feedback/input 
> here.



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

[GitHub] thrift issue #1461: THRIFT-4447: Golang: Fix panic on p.c.Call when using de...

[dcelasun]

Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1461
  
```go
type MyServiceClient struct {
c thrift.TClient
*MyServiceBaseClient
}

type MyServiceBaseClient struct {
c thrift.TClient
}
```

@johnboiles Are you getting this second "Base" struct with 0.11 compiler? 
Because I can't reproduce.

> Also should I remove the // Deprecated comments if these are not actually 
deprecated?

Yes please.


---

[jira] [Commented] (THRIFT-4447) Golang: Panic on p.c.Call when using deprecated initializers

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4447:


Github user johnboiles commented on the issue:

https://github.com/apache/thrift/pull/1461
  
@dcelasun any thoughts on fixing the tests? Looks like 
`tutorial/go/src/tutorial/tutorial.go` imports its `BaseClient` from another 
package. Since `c` isn't public in that other package, my modified initializer 
fails. 

Probably the simplest fix would just be to add a public accessor for `c`, 
(e.g. `func (p *BaseClient) C() thrift.TClient`) and us that public access in 
my modified initializer methods. What do you think?


> Golang: Panic on p.c.Call when using deprecated initializers
> 
>
> Key: THRIFT-4447
> URL: https://issues.apache.org/jira/browse/THRIFT-4447
> Project: Thrift
>  Issue Type: Bug
>  Components: Go - Compiler
>Affects Versions: 0.11.0
>Reporter: John Boiles
>
> Latest thrift:master can cause panics when using deprecated 
> `New*ClientFactory` and `New*ClientProtocol` functions. This happens because 
> both the Client and the BaseClient have an instance of a {{thrift.TClient}}. 
> The deprecated methods initialize the BaseClient's TClient, but other methods 
> use the Client's {{TClient}}.
> For example, current thrift master generates structs like this
> {code}
> type MyServiceClient struct {
>   c thrift.TClient
>   *MyServiceBaseClient
> }
> type MyServiceBaseClient struct {
>   c thrift.TClient
> }
> {code}
> And also a method like this:
> {code}
> func NewMyServiceClientFactory(t thrift.TTransport, f 
> thrift.TProtocolFactory) *MyServiceClient {
>   return {MyServiceBaseClient: 
> NewMyServiceBaseClientFactory(t, f)}
> }
> {code}
> If that method is used, later calls to service methods will panic, since 
> {{p.c}} is nil (the actual client was stored in {{p.BaseMyServiceClient.c}}).
> {code}
> func (p *MyServiceClient) DoStuff(ctx context.Context, request 
> *DoStuffRequest) (r *DoStuffResponse, err error) {
>   var _args139 DoStuffArgs
>   _args139.Request = request
>   var _result140 DoStuffResult
>   if err = p.c.Call(ctx, "do_stuff", &_args139, &_result140); err != nil 
> { // PANIC
>   ...
> {code}
> In progress fix here :https://github.com/apache/thrift/pull/1461. The fix in 
> this PR merely sets both instances of {{TClient}} (which is what happens in 
> the non-deprecated {{New*Client}} function).
> This patch currently fails {{make -k check}} however, since 
> {{src/tutorial/tutorial.go}} tries to access a different package's version of 
> the BaseClient.
> {code}
> src/tutorial/tutorial.go:477:33: bc.c undefined (cannot refer to unexported 
> field or method c)
> {code}
> The fix for that test could possibly be to expose the BaseClient's instance 
> of {{c}} (by making it a capital and thus exported {{C}}), or adding an 
> accessor method {{C()}} or {{Client()}}.
> Possibly a better fix would be to either remove these deprecated methods, or 
> figure out which {{TClient}} is the correct one to set.
> I'm not sure which is preferred, so I'm hoping to get some feedback/input 
> here.



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

[GitHub] thrift issue #1461: THRIFT-4447: Golang: Fix panic on p.c.Call when using de...

[johnboiles]

Github user johnboiles commented on the issue:

https://github.com/apache/thrift/pull/1461
  
@dcelasun any thoughts on fixing the tests? Looks like 
`tutorial/go/src/tutorial/tutorial.go` imports its `BaseClient` from another 
package. Since `c` isn't public in that other package, my modified initializer 
fails. 

Probably the simplest fix would just be to add a public accessor for `c`, 
(e.g. `func (p *BaseClient) C() thrift.TClient`) and us that public access in 
my modified initializer methods. What do you think?


---

[jira] [Commented] (THRIFT-4447) Golang: Panic on p.c.Call when using deprecated initializers

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4447:


Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1461
  
> Why are they deprecated? I probably only overlook sth, so bear with me

My apologies, it's an unfortunate result of the patch changing authors and 
things getting mixed up in the process. I should have removed the deprecated 
comments, especially since those constructors *are* compatible.

The bugfix looks good.


> Golang: Panic on p.c.Call when using deprecated initializers
> 
>
> Key: THRIFT-4447
> URL: https://issues.apache.org/jira/browse/THRIFT-4447
> Project: Thrift
>  Issue Type: Bug
>  Components: Go - Compiler
>Affects Versions: 0.11.0
>Reporter: John Boiles
>
> Latest thrift:master can cause panics when using deprecated 
> `New*ClientFactory` and `New*ClientProtocol` functions. This happens because 
> both the Client and the BaseClient have an instance of a {{thrift.TClient}}. 
> The deprecated methods initialize the BaseClient's TClient, but other methods 
> use the Client's {{TClient}}.
> For example, current thrift master generates structs like this
> {code}
> type MyServiceClient struct {
>   c thrift.TClient
>   *MyServiceBaseClient
> }
> type MyServiceBaseClient struct {
>   c thrift.TClient
> }
> {code}
> And also a method like this:
> {code}
> func NewMyServiceClientFactory(t thrift.TTransport, f 
> thrift.TProtocolFactory) *MyServiceClient {
>   return {MyServiceBaseClient: 
> NewMyServiceBaseClientFactory(t, f)}
> }
> {code}
> If that method is used, later calls to service methods will panic, since 
> {{p.c}} is nil (the actual client was stored in {{p.BaseMyServiceClient.c}}).
> {code}
> func (p *MyServiceClient) DoStuff(ctx context.Context, request 
> *DoStuffRequest) (r *DoStuffResponse, err error) {
>   var _args139 DoStuffArgs
>   _args139.Request = request
>   var _result140 DoStuffResult
>   if err = p.c.Call(ctx, "do_stuff", &_args139, &_result140); err != nil 
> { // PANIC
>   ...
> {code}
> In progress fix here :https://github.com/apache/thrift/pull/1461. The fix in 
> this PR merely sets both instances of {{TClient}} (which is what happens in 
> the non-deprecated {{New*Client}} function).
> This patch currently fails {{make -k check}} however, since 
> {{src/tutorial/tutorial.go}} tries to access a different package's version of 
> the BaseClient.
> {code}
> src/tutorial/tutorial.go:477:33: bc.c undefined (cannot refer to unexported 
> field or method c)
> {code}
> The fix for that test could possibly be to expose the BaseClient's instance 
> of {{c}} (by making it a capital and thus exported {{C}}), or adding an 
> accessor method {{C()}} or {{Client()}}.
> Possibly a better fix would be to either remove these deprecated methods, or 
> figure out which {{TClient}} is the correct one to set.
> I'm not sure which is preferred, so I'm hoping to get some feedback/input 
> here.



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

[GitHub] thrift issue #1461: THRIFT-4447: Golang: fix for (deprecated) New*ClientFact...

[dcelasun]

Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1461
  
> Why are they deprecated? I probably only overlook sth, so bear with me

My apologies, it's an unfortunate result of the patch changing authors and 
things getting mixed up in the process. I should have removed the deprecated 
comments, especially since those constructors *are* compatible.

The bugfix looks good.


---

[jira] [Created] (THRIFT-4448) Golang: do something with context.Context

[John Boiles (JIRA)]

John Boiles created THRIFT-4448:
---

 Summary: Golang: do something with context.Context
 Key: THRIFT-4448
 URL: https://issues.apache.org/jira/browse/THRIFT-4448
 Project: Thrift
  Issue Type: Task
  Components: Go - Library
Affects Versions: 0.11.0
Reporter: John Boiles


PR Here: https://github.com/apache/thrift/pull/1459

This patch wires through {{context.Context}} such that it can be used in in 
{{http.Request}}'s {{WithContext}} method. This allows Thrift HTTP requests to 
canceled or timed out via the context.

This patch breaks support for go<1.7 so it's not ready to ship, but I'm hoping 
to get some direction on this. When does Thrift expect to drop support of 
go1.7? It looks like the current solution is to duplicate files that need to 
use {{golang.org/x/net/context}} and add a {{// +build !go1.7}} but duplication 
seems unsustainable as the {{context}} package is imported more places.

Go 1.7 was released 15 August 2016. Given Golang has had significant 
performance improvements in most dot releases, I suspect most production 
services stay reasonably up to date. Here at Periscope/Twitter we're on 
go1.9.1, and we're a fairly large organization.



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

[GitHub] thrift issue #1461: Golang: fix for (deprecated) New*ClientFactory and New*C...

[johnboiles]

Github user johnboiles commented on the issue:

https://github.com/apache/thrift/pull/1461
  
> Why are they deprecated? I probably only overlook sth, so bear with me

That's a great question. Looks like they were marked deprecated by 
@dcelasun in #1382 as a part of THRIFT-4285. That discussion predates my 
involvement in Thrift.

> Do we have a JIRA Ticket for this? If not, could you create one?

Thanks for the nudge, I was just being lazy. Created as THRIFT-4447



---

[jira] [Commented] (THRIFT-4447) Golang: Panic on p.c.Call when using deprecated initializers

[John Boiles (JIRA)]

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

John Boiles commented on THRIFT-4447:
-

Looks like the regression came with https://github.com/apache/thrift/pull/1382

> Golang: Panic on p.c.Call when using deprecated initializers
> 
>
> Key: THRIFT-4447
> URL: https://issues.apache.org/jira/browse/THRIFT-4447
> Project: Thrift
>  Issue Type: Bug
>  Components: Go - Compiler
>Affects Versions: 0.11.0
>Reporter: John Boiles
>
> Latest thrift:master can cause panics when using deprecated 
> `New*ClientFactory` and `New*ClientProtocol` functions. This happens because 
> both the Client and the BaseClient have an instance of a {{thrift.TClient}}. 
> The deprecated methods initialize the BaseClient's TClient, but other methods 
> use the Client's {{TClient}}.
> For example, current thrift master generates structs like this
> {code}
> type MyServiceClient struct {
>   c thrift.TClient
>   *MyServiceBaseClient
> }
> type MyServiceBaseClient struct {
>   c thrift.TClient
> }
> {code}
> And also a method like this:
> {code}
> func NewMyServiceClientFactory(t thrift.TTransport, f 
> thrift.TProtocolFactory) *MyServiceClient {
>   return {MyServiceBaseClient: 
> NewMyServiceBaseClientFactory(t, f)}
> }
> {code}
> If that method is used, later calls to service methods will panic, since 
> {{p.c}} is nil (the actual client was stored in {{p.BaseMyServiceClient.c}}).
> {code}
> func (p *MyServiceClient) DoStuff(ctx context.Context, request 
> *DoStuffRequest) (r *DoStuffResponse, err error) {
>   var _args139 DoStuffArgs
>   _args139.Request = request
>   var _result140 DoStuffResult
>   if err = p.c.Call(ctx, "do_stuff", &_args139, &_result140); err != nil 
> { // PANIC
>   ...
> {code}
> In progress fix here :https://github.com/apache/thrift/pull/1461. The fix in 
> this PR merely sets both instances of {{TClient}} (which is what happens in 
> the non-deprecated {{New*Client}} function).
> This patch currently fails {{make -k check}} however, since 
> {{src/tutorial/tutorial.go}} tries to access a different package's version of 
> the BaseClient.
> {code}
> src/tutorial/tutorial.go:477:33: bc.c undefined (cannot refer to unexported 
> field or method c)
> {code}
> The fix for that test could possibly be to expose the BaseClient's instance 
> of {{c}} (by making it a capital and thus exported {{C}}), or adding an 
> accessor method {{C()}} or {{Client()}}.
> Possibly a better fix would be to either remove these deprecated methods, or 
> figure out which {{TClient}} is the correct one to set.
> I'm not sure which is preferred, so I'm hoping to get some feedback/input 
> here.



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

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4285:


Github user johnboiles commented on the issue:

https://github.com/apache/thrift/pull/1461
  
> Why are they deprecated? I probably only overlook sth, so bear with me

That's a great question. Looks like they were marked deprecated by 
@dcelasun in #1382 as a part of THRIFT-4285. That discussion predates my 
involvement in Thrift.

> Do we have a JIRA Ticket for this? If not, could you create one?

Thanks for the nudge, I was just being lazy. Created as THRIFT-4447



> 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
> 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] [Created] (THRIFT-4447) Golang: Panic on p.c.Call when using deprecated initializers

[John Boiles (JIRA)]

John Boiles created THRIFT-4447:
---

 Summary: Golang: Panic on p.c.Call when using deprecated 
initializers
 Key: THRIFT-4447
 URL: https://issues.apache.org/jira/browse/THRIFT-4447
 Project: Thrift
  Issue Type: Bug
  Components: Go - Compiler
Affects Versions: 0.11.0
Reporter: John Boiles


Latest thrift:master can cause panics when using deprecated `New*ClientFactory` 
and `New*ClientProtocol` functions. This happens because both the Client and 
the BaseClient have an instance of a {{thrift.TClient}}. The deprecated methods 
initialize the BaseClient's TClient, but other methods use the Client's 
{{TClient}}.

For example, current thrift master generates structs like this

{code}
type MyServiceClient struct {
c thrift.TClient
*MyServiceBaseClient
}

type MyServiceBaseClient struct {
c thrift.TClient
}
{code}

And also a method like this:

{code}
func NewMyServiceClientFactory(t thrift.TTransport, f thrift.TProtocolFactory) 
*MyServiceClient {
  return {MyServiceBaseClient: 
NewMyServiceBaseClientFactory(t, f)}
}
{code}

If that method is used, later calls to service methods will panic, since 
{{p.c}} is nil (the actual client was stored in {{p.BaseMyServiceClient.c}}).

{code}
func (p *MyServiceClient) DoStuff(ctx context.Context, request *DoStuffRequest) 
(r *DoStuffResponse, err error) {
var _args139 DoStuffArgs
_args139.Request = request
var _result140 DoStuffResult
if err = p.c.Call(ctx, "do_stuff", &_args139, &_result140); err != nil 
{ // PANIC
...
{code}

In progress fix here :https://github.com/apache/thrift/pull/1461. The fix in 
this PR merely sets both instances of {{TClient}} (which is what happens in the 
non-deprecated {{New*Client}} function).

This patch currently fails {{make -k check}} however, since 
{{src/tutorial/tutorial.go}} tries to access a different package's version of 
the BaseClient.

{code}
src/tutorial/tutorial.go:477:33: bc.c undefined (cannot refer to unexported 
field or method c)
{code}

The fix for that test could possibly be to expose the BaseClient's instance of 
{{c}} (by making it a capital and thus exported {{C}}), or adding an accessor 
method {{C()}} or {{Client()}}.

Possibly a better fix would be to either remove these deprecated methods, or 
figure out which {{TClient}} is the correct one to set.

I'm not sure which is preferred, so I'm hoping to get some feedback/input here.



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

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4285:


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

https://github.com/apache/thrift/pull/1382#discussion_r160477076
  
--- Diff: compiler/cpp/src/thrift/generate/t_go_generator.cc ---
@@ -1953,177 +1960,75 @@ void 
t_go_generator::generate_service_client(t_service* tservice) {
 f_types_ << indent() << "func (p *" << serviceName << "Client) "
<< function_signature_if(*f_iter, "", true) << " {" << endl;
 indent_up();
-/*
-f_types_ <<
-  indent() << "p.SeqId += 1" << endl;
-if (!(*f_iter)->is_oneway()) {
-  f_types_ <<
-indent() << "d := defer.Deferred()" << endl <<
-indent() << "p.Reqs[p.SeqId] = d" << endl;
-}
-*/
-f_types_ << indent() << "if err = p.send" << funname << "(";
-bool first = true;
-
-for (fld_iter = fields.begin(); fld_iter != fields.end(); ++fld_iter) {
-  if (first) {
-first = false;
-  } else {
-f_types_ << ", ";
-  }
 
-  f_types_ << variable_name_to_go_name((*fld_iter)->get_name());
-}
-
-f_types_ << "); err != nil { return }" << endl;
-
-if (!(*f_iter)->is_oneway()) {
-  f_types_ << indent() << "return p.recv" << funname << "()" << endl;
-} else {
-  f_types_ << indent() << "return" << endl;
-}
-
-indent_down();
-f_types_ << indent() << "}" << endl << endl;
-f_types_ << indent() << "func (p *" << serviceName << "Client) send"
-   << function_signature(*f_iter) << "(err error) {" << endl;
-indent_up();
-std::string argsname = publicize((*f_iter)->get_name() + "_args", 
true);
-// Serialize the request header
-f_types_ << indent() << "oprot := p.OutputProtocol" << endl;
-f_types_ << indent() << "if oprot == nil {" << endl;
-f_types_ << indent() << "  oprot = 
p.ProtocolFactory.GetProtocol(p.Transport)" << endl;
-f_types_ << indent() << "  p.OutputProtocol = oprot" << endl;
-f_types_ << indent() << "}" << endl;
-f_types_ << indent() << "p.SeqId++" << endl;
-f_types_ << indent() << "if err = oprot.WriteMessageBegin(\"" << 
(*f_iter)->get_name()
-   << "\", " << ((*f_iter)->is_oneway() ? "thrift.ONEWAY" : 
"thrift.CALL")
-   << ", p.SeqId); err != nil {" << endl;
-indent_up();
-f_types_ << indent() << "  return" << endl;
-indent_down();
-f_types_ << indent() << "}" << endl;
-f_types_ << indent() << "args := " << argsname << "{" << endl;
+std::string method = (*f_iter)->get_name();
+std::string argsType = publicize(method + "_args", true);
+std::string argsName = tmp("_args");
+f_types_ << indent() << "var " << argsName << " " << argsType << endl;
 
 for (fld_iter = fields.begin(); fld_iter != fields.end(); ++fld_iter) {
-  f_types_ << indent() << publicize((*fld_iter)->get_name()) << " : "
- << variable_name_to_go_name((*fld_iter)->get_name()) << 
"," << endl;
+  f_types_ << indent() << argsName << "." << 
publicize((*fld_iter)->get_name())
+   << " = " << 
variable_name_to_go_name((*fld_iter)->get_name()) << endl;
 }
-f_types_ << indent() << "}" << endl;
-
-// Write to the stream
-f_types_ << indent() << "if err = args." << write_method_name_ << 
"(oprot); err != nil {" << endl;
-indent_up();
-f_types_ << indent() << "  return" << endl;
-indent_down();
-f_types_ << indent() << "}" << endl;
-f_types_ << indent() << "if err = oprot.WriteMessageEnd(); err != nil 
{" << endl;
-indent_up();
-f_types_ << indent() << "  return" << endl;
-indent_down();
-f_types_ << indent() << "}" << endl;
-f_types_ << indent() << "return oprot.Flush()" << endl;
-indent_down();
-f_types_ << indent() << "}" << endl << endl;
 
 if (!(*f_iter)->is_oneway()) {
-  std::string resultname = publicize((*f_iter)->get_name() + 
"_result", true);
-  // Open function
-  f_types_ << endl << indent() << "func (p *" << serviceName << 
"Client) recv"
- << publicize((*f_iter)->get_name()) << "() (";
+  std::string resultName = tmp("_result");
+  std::string resultType = publicize(method + "_result", true);
+  f_types_ << indent() << "var " << resultName << " " << resultType << 
endl;
+  f_types_ << indent() << "if err = p.c.Call(ctx, \""
--- End diff --

When using one of the now-deprecated 

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

[johnboiles]

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

https://github.com/apache/thrift/pull/1382#discussion_r160477076
  
--- Diff: compiler/cpp/src/thrift/generate/t_go_generator.cc ---
@@ -1953,177 +1960,75 @@ void 
t_go_generator::generate_service_client(t_service* tservice) {
 f_types_ << indent() << "func (p *" << serviceName << "Client) "
<< function_signature_if(*f_iter, "", true) << " {" << endl;
 indent_up();
-/*
-f_types_ <<
-  indent() << "p.SeqId += 1" << endl;
-if (!(*f_iter)->is_oneway()) {
-  f_types_ <<
-indent() << "d := defer.Deferred()" << endl <<
-indent() << "p.Reqs[p.SeqId] = d" << endl;
-}
-*/
-f_types_ << indent() << "if err = p.send" << funname << "(";
-bool first = true;
-
-for (fld_iter = fields.begin(); fld_iter != fields.end(); ++fld_iter) {
-  if (first) {
-first = false;
-  } else {
-f_types_ << ", ";
-  }
 
-  f_types_ << variable_name_to_go_name((*fld_iter)->get_name());
-}
-
-f_types_ << "); err != nil { return }" << endl;
-
-if (!(*f_iter)->is_oneway()) {
-  f_types_ << indent() << "return p.recv" << funname << "()" << endl;
-} else {
-  f_types_ << indent() << "return" << endl;
-}
-
-indent_down();
-f_types_ << indent() << "}" << endl << endl;
-f_types_ << indent() << "func (p *" << serviceName << "Client) send"
-   << function_signature(*f_iter) << "(err error) {" << endl;
-indent_up();
-std::string argsname = publicize((*f_iter)->get_name() + "_args", 
true);
-// Serialize the request header
-f_types_ << indent() << "oprot := p.OutputProtocol" << endl;
-f_types_ << indent() << "if oprot == nil {" << endl;
-f_types_ << indent() << "  oprot = 
p.ProtocolFactory.GetProtocol(p.Transport)" << endl;
-f_types_ << indent() << "  p.OutputProtocol = oprot" << endl;
-f_types_ << indent() << "}" << endl;
-f_types_ << indent() << "p.SeqId++" << endl;
-f_types_ << indent() << "if err = oprot.WriteMessageBegin(\"" << 
(*f_iter)->get_name()
-   << "\", " << ((*f_iter)->is_oneway() ? "thrift.ONEWAY" : 
"thrift.CALL")
-   << ", p.SeqId); err != nil {" << endl;
-indent_up();
-f_types_ << indent() << "  return" << endl;
-indent_down();
-f_types_ << indent() << "}" << endl;
-f_types_ << indent() << "args := " << argsname << "{" << endl;
+std::string method = (*f_iter)->get_name();
+std::string argsType = publicize(method + "_args", true);
+std::string argsName = tmp("_args");
+f_types_ << indent() << "var " << argsName << " " << argsType << endl;
 
 for (fld_iter = fields.begin(); fld_iter != fields.end(); ++fld_iter) {
-  f_types_ << indent() << publicize((*fld_iter)->get_name()) << " : "
- << variable_name_to_go_name((*fld_iter)->get_name()) << 
"," << endl;
+  f_types_ << indent() << argsName << "." << 
publicize((*fld_iter)->get_name())
+   << " = " << 
variable_name_to_go_name((*fld_iter)->get_name()) << endl;
 }
-f_types_ << indent() << "}" << endl;
-
-// Write to the stream
-f_types_ << indent() << "if err = args." << write_method_name_ << 
"(oprot); err != nil {" << endl;
-indent_up();
-f_types_ << indent() << "  return" << endl;
-indent_down();
-f_types_ << indent() << "}" << endl;
-f_types_ << indent() << "if err = oprot.WriteMessageEnd(); err != nil 
{" << endl;
-indent_up();
-f_types_ << indent() << "  return" << endl;
-indent_down();
-f_types_ << indent() << "}" << endl;
-f_types_ << indent() << "return oprot.Flush()" << endl;
-indent_down();
-f_types_ << indent() << "}" << endl << endl;
 
 if (!(*f_iter)->is_oneway()) {
-  std::string resultname = publicize((*f_iter)->get_name() + 
"_result", true);
-  // Open function
-  f_types_ << endl << indent() << "func (p *" << serviceName << 
"Client) recv"
- << publicize((*f_iter)->get_name()) << "() (";
+  std::string resultName = tmp("_result");
+  std::string resultType = publicize(method + "_result", true);
+  f_types_ << indent() << "var " << resultName << " " << resultType << 
endl;
+  f_types_ << indent() << "if err = p.c.Call(ctx, \""
--- End diff --

When using one of the now-deprecated initializers, this line will cause a 
panic, since `p.c` is nil. The deprecated initializers create the `TClient` at 
`p.BaseMyServiceClient.c`


---

[jira] [Created] (THRIFT-4446) JSONProtocol Base64 Encoding Trims Padding

[Allen (JIRA)]

Allen created THRIFT-4446:
-

 Summary: JSONProtocol Base64 Encoding Trims Padding
 Key: THRIFT-4446
 URL: https://issues.apache.org/jira/browse/THRIFT-4446
 Project: Thrift
  Issue Type: Bug
  Components: .NETCore - Library, C# - Library
Affects Versions: 0.11.0
Reporter: Allen


In the C# and .NET Core libraries, the JSONProtocol's Binary Encoding to Base64 
trims padding from the user provided byte arrays before encoding into Base64. 
This behavior is incorrect, as the user provided data should be encoded exactly 
as provided. Otherwise, data may be lost.

Fixed by no longer trimming padding on encode. Padding must still be trimmed on 
decode, in accordance with the Base64 specification.

For example:
* Before this patch, encoding the byte array [0x01, 0x3d, 0x3d] yields [0x01] 
upon decode. This is incorrect, as I should decode the exact data that I 
encoded.
* After this patch, it yields [0x01, 0x3d, 0x3d], as expected.

I have submitted a pull request 
[here|https://github.com/apache/thrift/pull/1463]



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

[GitHub] thrift pull request #1463: JSONProtocol Base64 Encoding: Do not trim padding...

[sithas]

GitHub user sithas opened a pull request:

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

JSONProtocol Base64 Encoding: Do not trim padding on encode.

* In the C# and .NET Core libraries, the JSONProtocol's Binary Encoding to 
Base64 trims padding from the user provided byte arrays before encoding into 
Base64. This behavior is incorrect, as the user provided data should be encoded 
exactly as provided. Otherwise, data may be lost.

* Fixed by no longer trimming padding on encode. Padding must still be 
trimmed on decode, in accordance with the Base64 specification.

* For example:
  * Before this patch, encoding the byte array `[0x01, 0x3d, 0x3d]` yields 
`[0x01]` upon decode. This is incorrect, as I should decode the exact data that 
I encoded.
  * After this patch, it yields `[0x01, 0x3d, 0x3d]`, as expected.

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

$ git pull https://github.com/sithas/thrift master

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

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


commit 4cdf1f4a332d5f14923fdc2929321664de1d3c8d
Author: Allen Warthen 
Date:   2018-01-09T17:04:14Z

JSONProtocol Base64 Encoding: Do not trim padding on encode.

- In the C# and .NET Core libraries, the JSONProtocol's Binary Encoding
to Base64 trims padding from the user provided byte arrays before
encoding into base64. This behaviour is incorrect, as the user provided
bytes should be encoded exactly as they are provided. Otherwise, data
may be lost.
- Fixed by no longer trimming padding on encode.




---

[GitHub] thrift issue #1461: Golang: fix for (deprecated) New*ClientFactory and New*C...

[Jens-G]

Github user Jens-G commented on the issue:

https://github.com/apache/thrift/pull/1461
  
Hi John,

Questions:
* Why are they deprecated? I probably only overlook sth, so bear with me
* Do we have a JIRA Ticket for this? If not, could you create one?





---