[GitHub] thrift issue #1516: THRIFT-4527 Upgrade byteorder in rust

2018-03-23 Thread allengeorge
Github user allengeorge commented on the issue:

https://github.com/apache/thrift/pull/1516
  
OK. Looks like the build failures are unrelated. I'm going to apply locally 
and check cross-tests just to make sure, but will get this applied today. Thank 
you @QuestofIranon! Always happy to see people improving Thrift :)


---


[jira] [Commented] (THRIFT-4527) Upgrade byteorder version in Rust lib

2018-03-23 Thread ASF GitHub Bot (JIRA)

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

ASF GitHub Bot commented on THRIFT-4527:


Github user allengeorge commented on the issue:

https://github.com/apache/thrift/pull/1516
  
OK. Looks like the build failures are unrelated. I'm going to apply locally 
and check cross-tests just to make sure, but will get this applied today. Thank 
you @QuestofIranon! Always happy to see people improving Thrift :)


> Upgrade byteorder version in Rust lib
> -
>
> Key: THRIFT-4527
> URL: https://issues.apache.org/jira/browse/THRIFT-4527
> Project: Thrift
>  Issue Type: Dependency upgrade
>  Components: Rust - Library
>Affects Versions: 0.11.0
>Reporter: Joshua
>Priority: Major
>
> The Rust library was depending on byteorder v1.1.0, upgraded to v1.2.1 to 
> prevent dependency conflicts in a project I was working on, works fine and 
> would be useful for other users.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] thrift pull request #1516: THRIFT-4527 Upgrade byteorder in rust

2018-03-23 Thread asfgit
Github user asfgit closed the pull request at:

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


---


[jira] [Commented] (THRIFT-4527) Upgrade byteorder version in Rust lib

2018-03-23 Thread ASF GitHub Bot (JIRA)

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

ASF GitHub Bot commented on THRIFT-4527:


Github user asfgit closed the pull request at:

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


> Upgrade byteorder version in Rust lib
> -
>
> Key: THRIFT-4527
> URL: https://issues.apache.org/jira/browse/THRIFT-4527
> Project: Thrift
>  Issue Type: Dependency upgrade
>  Components: Rust - Library
>Affects Versions: 0.11.0
>Reporter: Joshua
>Priority: Major
>
> The Rust library was depending on byteorder v1.1.0, upgraded to v1.2.1 to 
> prevent dependency conflicts in a project I was working on, works fine and 
> would be useful for other users.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Created] (THRIFT-4530) proposal: add nullability annotations to generated Java code

2018-03-23 Thread Manu Sridharan (JIRA)
Manu Sridharan created THRIFT-4530:
--

 Summary: proposal: add nullability annotations to generated Java 
code
 Key: THRIFT-4530
 URL: https://issues.apache.org/jira/browse/THRIFT-4530
 Project: Thrift
  Issue Type: New Feature
  Components: Java - Compiler
Reporter: Manu Sridharan


I'd like to propose (optionally) including {{@Nullable}} annotations in 
Thrift-generated Java code.  I'm the main author of NullAway and we'd like to 
better support users who are using Thrift.  The change would involve changing 
the Java code generator to include {{@Nullable}} annotations on every field, 
method return value, and method parameter in the public API that may be null.  
With these annotations, NullAway users will get warnings when their client code 
is missing appropriate null checks.  Also, IDEs like IntelliJ will give better 
warnings about missing null checks.  As part of this change, I would also add 
support to NullAway for understanding {{isSetX()}} methods to avoid excessive 
false positives.

Regarding which {{@Nullable}} annotation to use, Thrift seems to try to 
minimize third-party dependencies, but we could simply include a new Thrift 
{{@Nullable}} annotation, and it will work fine with NullAway and most other 
tools.

I have a WIP patch to generate these annotations, but I wanted to get feedback 
from the maintainers before opening a PR.  We could of course make the 
annotation generation optional and default it to being off, if desired.  
Anyway, thoughts / feedback welcome.  Thanks!



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (THRIFT-4530) proposal: add nullability annotations to generated Java code

2018-03-23 Thread Manu Sridharan (JIRA)

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

Manu Sridharan updated THRIFT-4530:
---
Description: 
I'd like to propose (optionally) including {{@Nullable}} annotations in 
Thrift-generated Java code.  I'm the main author of NullAway 
([https://github.com/uber/NullAway)] and we'd like to better support users who 
are using Thrift.  The change would involve changing the Java code generator to 
include {{@Nullable}} annotations on every field, method return value, and 
method parameter in the public API that may be null.  With these annotations, 
NullAway users will get warnings when their client code is missing appropriate 
null checks.  Also, IDEs like IntelliJ will give better warnings about missing 
null checks.  As part of this change, I would also add support to NullAway for 
understanding {{isSetX()}} methods to avoid excessive false positives.

Regarding which {{@Nullable}} annotation to use, Thrift seems to try to 
minimize third-party dependencies, but we could simply include a new Thrift 
{{@Nullable}} annotation, and it will work fine with NullAway and most other 
tools.

I have a WIP patch to generate these annotations, but I wanted to get feedback 
from the maintainers before opening a PR.  We could of course make the 
annotation generation optional and default it to being off, if desired.  
Anyway, thoughts / feedback welcome.  Thanks!

  was:
I'd like to propose (optionally) including {{@Nullable}} annotations in 
Thrift-generated Java code.  I'm the main author of NullAway and we'd like to 
better support users who are using Thrift.  The change would involve changing 
the Java code generator to include {{@Nullable}} annotations on every field, 
method return value, and method parameter in the public API that may be null.  
With these annotations, NullAway users will get warnings when their client code 
is missing appropriate null checks.  Also, IDEs like IntelliJ will give better 
warnings about missing null checks.  As part of this change, I would also add 
support to NullAway for understanding {{isSetX()}} methods to avoid excessive 
false positives.

Regarding which {{@Nullable}} annotation to use, Thrift seems to try to 
minimize third-party dependencies, but we could simply include a new Thrift 
{{@Nullable}} annotation, and it will work fine with NullAway and most other 
tools.

I have a WIP patch to generate these annotations, but I wanted to get feedback 
from the maintainers before opening a PR.  We could of course make the 
annotation generation optional and default it to being off, if desired.  
Anyway, thoughts / feedback welcome.  Thanks!


> proposal: add nullability annotations to generated Java code
> 
>
> Key: THRIFT-4530
> URL: https://issues.apache.org/jira/browse/THRIFT-4530
> Project: Thrift
>  Issue Type: New Feature
>  Components: Java - Compiler
>Reporter: Manu Sridharan
>Priority: Major
>
> I'd like to propose (optionally) including {{@Nullable}} annotations in 
> Thrift-generated Java code.  I'm the main author of NullAway 
> ([https://github.com/uber/NullAway)] and we'd like to better support users 
> who are using Thrift.  The change would involve changing the Java code 
> generator to include {{@Nullable}} annotations on every field, method return 
> value, and method parameter in the public API that may be null.  With these 
> annotations, NullAway users will get warnings when their client code is 
> missing appropriate null checks.  Also, IDEs like IntelliJ will give better 
> warnings about missing null checks.  As part of this change, I would also add 
> support to NullAway for understanding {{isSetX()}} methods to avoid excessive 
> false positives.
> Regarding which {{@Nullable}} annotation to use, Thrift seems to try to 
> minimize third-party dependencies, but we could simply include a new Thrift 
> {{@Nullable}} annotation, and it will work fine with NullAway and most other 
> tools.
> I have a WIP patch to generate these annotations, but I wanted to get 
> feedback from the maintainers before opening a PR.  We could of course make 
> the annotation generation optional and default it to being off, if desired.  
> Anyway, thoughts / feedback welcome.  Thanks!



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (THRIFT-4530) proposal: add nullability annotations to generated Java code

2018-03-23 Thread Manu Sridharan (JIRA)

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

Manu Sridharan updated THRIFT-4530:
---
Description: 
I'd like to propose (optionally) including {{@Nullable}} annotations in 
Thrift-generated Java code.  I'm the main author of NullAway 
([https://github.com/uber/NullAway)] and we'd like to better support users who 
are using Thrift.  The change would involve changing the Java code generator to 
include {{@Nullable}} annotations on every field, method return value, and 
method parameter in the public API of generated code that may be null.  With 
these annotations, NullAway users will get warnings when their client code is 
missing appropriate null checks.  Also, IDEs like IntelliJ will give better 
warnings about missing null checks.  As part of this change, I would also add 
support to NullAway for understanding {{isSetX()}} methods to avoid excessive 
false positives.

Regarding which {{@Nullable}} annotation to use, Thrift seems to try to 
minimize third-party dependencies, but we could simply include a new Thrift 
{{@Nullable}} annotation, and it will work fine with NullAway and most other 
tools.

I have a WIP patch to generate these annotations, but I wanted to get feedback 
from the maintainers before opening a PR.  We could of course make the 
annotation generation optional and default it to being off, if desired.  
Anyway, thoughts / feedback welcome.  Thanks!

  was:
I'd like to propose (optionally) including {{@Nullable}} annotations in 
Thrift-generated Java code.  I'm the main author of NullAway 
([https://github.com/uber/NullAway)] and we'd like to better support users who 
are using Thrift.  The change would involve changing the Java code generator to 
include {{@Nullable}} annotations on every field, method return value, and 
method parameter in the public API that may be null.  With these annotations, 
NullAway users will get warnings when their client code is missing appropriate 
null checks.  Also, IDEs like IntelliJ will give better warnings about missing 
null checks.  As part of this change, I would also add support to NullAway for 
understanding {{isSetX()}} methods to avoid excessive false positives.

Regarding which {{@Nullable}} annotation to use, Thrift seems to try to 
minimize third-party dependencies, but we could simply include a new Thrift 
{{@Nullable}} annotation, and it will work fine with NullAway and most other 
tools.

I have a WIP patch to generate these annotations, but I wanted to get feedback 
from the maintainers before opening a PR.  We could of course make the 
annotation generation optional and default it to being off, if desired.  
Anyway, thoughts / feedback welcome.  Thanks!


> proposal: add nullability annotations to generated Java code
> 
>
> Key: THRIFT-4530
> URL: https://issues.apache.org/jira/browse/THRIFT-4530
> Project: Thrift
>  Issue Type: New Feature
>  Components: Java - Compiler
>Reporter: Manu Sridharan
>Priority: Major
>
> I'd like to propose (optionally) including {{@Nullable}} annotations in 
> Thrift-generated Java code.  I'm the main author of NullAway 
> ([https://github.com/uber/NullAway)] and we'd like to better support users 
> who are using Thrift.  The change would involve changing the Java code 
> generator to include {{@Nullable}} annotations on every field, method return 
> value, and method parameter in the public API of generated code that may be 
> null.  With these annotations, NullAway users will get warnings when their 
> client code is missing appropriate null checks.  Also, IDEs like IntelliJ 
> will give better warnings about missing null checks.  As part of this change, 
> I would also add support to NullAway for understanding {{isSetX()}} methods 
> to avoid excessive false positives.
> Regarding which {{@Nullable}} annotation to use, Thrift seems to try to 
> minimize third-party dependencies, but we could simply include a new Thrift 
> {{@Nullable}} annotation, and it will work fine with NullAway and most other 
> tools.
> I have a WIP patch to generate these annotations, but I wanted to get 
> feedback from the maintainers before opening a PR.  We could of course make 
> the annotation generation optional and default it to being off, if desired.  
> Anyway, thoughts / feedback welcome.  Thanks!



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Resolved] (THRIFT-4342) Support ruby rspec 3

2018-03-23 Thread James E. King, III (JIRA)

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

James E. King, III resolved THRIFT-4342.

   Resolution: Fixed
Fix Version/s: 0.12.0

> Support ruby rspec 3
> 
>
> Key: THRIFT-4342
> URL: https://issues.apache.org/jira/browse/THRIFT-4342
> Project: Thrift
>  Issue Type: Improvement
>  Components: Ruby - Library
>Affects Versions: 0.10.0
> Environment: docker ubuntu-xenial (ruby 2.3.1)
>Reporter: James E. King, III
>Assignee: James E. King, III
>Priority: Major
> Fix For: 0.12.0
>
>
> I don't know much about ruby.  Anybody who can assist is welcome to do so!  I 
> tried to update us to rspec 2.99.x to prepare for rspec 3 and got a lot of 
> errors.  We should be running the latest versions of test tools in 
> thrift.gemspec.  Also note the apache license in the gemspec is not quite 
> right...
> {noformat}
> Pending:
>   Client Thrift::Client should increment the sequence id when sending messages
> # it seems sequence ids are completely ignored right now
> # ./spec/client_spec.rb:55
> Failures:
>   1) BinaryProtocolAccelerated it should behave like a binary protocol should 
> write a byte
>  Failure/Error: @trans.rspec_verify
>  NoMethodError:
>undefined method `rspec_verify' for 
> #
>  Shared Example Group: "a binary protocol" called from 
> ./spec/binary_protocol_accelerated_spec.rb:28
>  # ./spec/binary_protocol_spec_shared.rb:112:in `block (2 levels) in  (required)>'
>   2) BinaryProtocol it should behave like a binary protocol should write a 
> byte
>  Failure/Error: @trans.rspec_verify
>  NoMethodError:
>undefined method `rspec_verify' for 
> #
>  Shared Example Group: "a binary protocol" called from 
> ./spec/binary_protocol_spec.rb:25
>  # ./spec/binary_protocol_spec_shared.rb:112:in `block (2 levels) in  (required)>'
> Deprecation Warnings:
> 
> The semantics of `RSpec::Core::Pending#pending` are changing in
> RSpec 3.  In RSpec 2.x, it caused the example to be skipped. In
> RSpec 3, the rest of the example will still be run but is expected
> to fail, and will be marked as a failure (rather than as pending)
> if the example passes.
> Any passed block will no longer be executed. This feature is being
> removed since it was semantically inconsistent, and the behaviour it
> offered is being made available with the other ways of marking an
> example pending.
> To keep the same skip semantics, change `pending` to `skip`.
> Otherwise, if you want the new RSpec 3 behavior, you can safely
> ignore this warning and continue to upgrade to RSpec 3 without
> addressing it.
> Called from /thrift/src/lib/rb/spec/client_spec.rb:56:in `block (3 levels) in 
> '.
> 
> `and_return { value }` is deprecated. Use `and_return(value)` or an 
> implementation block without `and_return` instead. Called from 
> /thrift/src/lib/rb/spec/client_spec.rb:79:in `block (3 levels) in  (required)>'.
> `and_return { value }` is deprecated. Use `and_return(value)` or an 
> implementation block without `and_return` instead. Called from 
> /thrift/src/lib/rb/spec/http_client_spec.rb:38:in `block (3 levels) in  (required)>'.
> `and_return { value }` is deprecated. Use `and_return(value)` or an 
> implementation block without `and_return` instead. Called from 
> /thrift/src/lib/rb/spec/http_client_spec.rb:41:in `block (5 levels) in  (required)>'.
> Too many uses of deprecated '`and_return { value }`'. Pass 
> `--deprecation-out` or set `config.deprecation_stream` to a file for full 
> output.
> `be_false` is deprecated. Use `be_falsey` (for Ruby's conditional semantics) 
> or `be false` (for exact `== false` equality) instead. Called from 
> /thrift/src/lib/rb/spec/base_transport_spec.rb:279:in `block (3 levels) in 
> '.
> `be_false` is deprecated. Use `be_falsey` (for Ruby's conditional semantics) 
> or `be false` (for exact `== false` equality) instead. Called from 
> /thrift/src/lib/rb/spec/struct_spec.rb:63:in `block (3 levels) in  (required)>'.
> `be_false` is deprecated. Use `be_falsey` (for Ruby's conditional semantics) 
> or `be false` (for exact `== false` equality) instead. Called from 
> /thrift/src/lib/rb/spec/struct_spec.rb:83:in `block (3 levels) in  (required)>'.
> Too many uses of deprecated '`be_false`'. Pass `--deprecation-out` or set 
> `config.deprecation_stream` to a file for full output.
> `be_true` is deprecated. Use `be_truthy` (for Ruby's conditional semantics) 
> or `be true` (for exact `== true` equality) instead. Called from 
> /thrift/src/lib/rb/spec/base_transport_spec.rb:44:in `block (4 levels) in 
> '.
> `be_true` is deprecated. Us

[jira] [Updated] (THRIFT-4482) error making c++ library on windows 8.1 using cygwin

2018-03-23 Thread James E. King, III (JIRA)

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

James E. King, III updated THRIFT-4482:
---
Priority: Trivial  (was: Major)

> error making c++ library on windows 8.1 using cygwin
> 
>
> Key: THRIFT-4482
> URL: https://issues.apache.org/jira/browse/THRIFT-4482
> Project: Thrift
>  Issue Type: Bug
>  Components: Build Process, C++ - Library
>Affects Versions: 0.11.0
> Environment: Windows 8.1, thrift 0.11.0
> cygwin64, boost 1_66, GNU make 4.2.1, g++ 6.4.0, automake 1.15.1, autoconf 
> 2.69
>Reporter: Michel Max
>Assignee: James E. King, III
>Priority: Trivial
>  Labels: build, windows
>
> src/thrift/transport/THttpServer.cpp: In member function 'virtual void 
> apache::thrift::transport::THttpServer::parseHeader(char*)':
> src/thrift/transport/THttpServer.cpp:50:74: error: 'strcasestr' was not 
> declared in this scope
>    #define THRIFT_strcasestr(haystack, needle) strcasestr(haystack, needle)
>   ^
> src/thrift/transport/THttpServer.cpp:62:9: note: in expansion of macro 
> 'THRIFT_strcasestr'
>  if (THRIFT_strcasestr(value, "chunked") != NULL) {
>  ^
> make[4]: *** [Makefile:1350: src/thrift/transport/THttpServer.lo] Error 1
>  
> This is a part of the console output, but it is the entire error. This 
> happened during the execution of make in the root directory.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (THRIFT-4482) error making c++ library on windows 8.1 using cygwin

2018-03-23 Thread James E. King, III (JIRA)

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

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


Is this still an issue for you?  Did you work around it, and if so how?

> error making c++ library on windows 8.1 using cygwin
> 
>
> Key: THRIFT-4482
> URL: https://issues.apache.org/jira/browse/THRIFT-4482
> Project: Thrift
>  Issue Type: Bug
>  Components: Build Process, C++ - Library
>Affects Versions: 0.11.0
> Environment: Windows 8.1, thrift 0.11.0
> cygwin64, boost 1_66, GNU make 4.2.1, g++ 6.4.0, automake 1.15.1, autoconf 
> 2.69
>Reporter: Michel Max
>Assignee: James E. King, III
>Priority: Trivial
>  Labels: build, windows
>
> src/thrift/transport/THttpServer.cpp: In member function 'virtual void 
> apache::thrift::transport::THttpServer::parseHeader(char*)':
> src/thrift/transport/THttpServer.cpp:50:74: error: 'strcasestr' was not 
> declared in this scope
>    #define THRIFT_strcasestr(haystack, needle) strcasestr(haystack, needle)
>   ^
> src/thrift/transport/THttpServer.cpp:62:9: note: in expansion of macro 
> 'THRIFT_strcasestr'
>  if (THRIFT_strcasestr(value, "chunked") != NULL) {
>  ^
> make[4]: *** [Makefile:1350: src/thrift/transport/THttpServer.lo] Error 1
>  
> This is a part of the console output, but it is the entire error. This 
> happened during the execution of make in the root directory.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (THRIFT-3118) Python MemoryError in THttpClient when using an SSL endpoint

2018-03-23 Thread James E. King, III (JIRA)

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

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


Was going to close this out but I see that we don't test "http" transport in 
the cross test, so I am going to add it.  As for the actual fix here, I think 
more information is needed such as what was the "sz" in question; what exact 
version of Python was in use?  I see it was a 2.7.x.  I would have expected 
Python to return a smaller data buffer if there isn't enough memory available 
for the whole buffer...

> Python MemoryError in THttpClient when using an SSL endpoint
> 
>
> Key: THRIFT-3118
> URL: https://issues.apache.org/jira/browse/THRIFT-3118
> Project: Thrift
>  Issue Type: Bug
>  Components: Python - Library
>Affects Versions: 0.9
>Reporter: Jean-Baptiste Quenot
>Assignee: James E. King, III
>Priority: Major
>
> When using the Python Thrift HTTP client, a MemoryError is raised by the 
> underlying socket when using SSL:
> {noformat}
> File 
> "/path/to/local/lib/python2.7/site-packages/thrift/transport/THttpClient.py", 
> line 116, in read
>   return self.__http.file.read(sz)
> File "/usr/lib/python2.7/socket.py", line 380, in read
>   data = self._sock.recv(left)
> File "/usr/lib/python2.7/ssl.py", line 241, in recv
>   return self.read(buflen)
> File "/usr/lib/python2.7/ssl.py", line 160, in read
>   return self._sslobj.read(len)
> MemoryError
> {noformat}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (THRIFT-3118) Python MemoryError in THttpClient when using an SSL endpoint

2018-03-23 Thread James E. King, III (JIRA)

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

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


I have http working, but https is another story.  Looks like we need to teach 
THttpClient how to work with certificates.

> Python MemoryError in THttpClient when using an SSL endpoint
> 
>
> Key: THRIFT-3118
> URL: https://issues.apache.org/jira/browse/THRIFT-3118
> Project: Thrift
>  Issue Type: Bug
>  Components: Python - Library
>Affects Versions: 0.9
>Reporter: Jean-Baptiste Quenot
>Assignee: James E. King, III
>Priority: Major
>
> When using the Python Thrift HTTP client, a MemoryError is raised by the 
> underlying socket when using SSL:
> {noformat}
> File 
> "/path/to/local/lib/python2.7/site-packages/thrift/transport/THttpClient.py", 
> line 116, in read
>   return self.__http.file.read(sz)
> File "/usr/lib/python2.7/socket.py", line 380, in read
>   data = self._sock.recv(left)
> File "/usr/lib/python2.7/ssl.py", line 241, in recv
>   return self.read(buflen)
> File "/usr/lib/python2.7/ssl.py", line 160, in read
>   return self._sslobj.read(len)
> MemoryError
> {noformat}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (THRIFT-4530) proposal: add nullability annotations to generated Java code

2018-03-23 Thread Can Celasun (JIRA)

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

Can Celasun commented on THRIFT-4530:
-

I think it's a good idea. Regarding which annotation to use, I think we should 
definitely include a default {{@Nullable}} implementation, but it might also be 
worth adding a compiler flag to use a different one.

For the patch, it's better if you send it as a PR on Github instead of posting 
it here, as a PR will trigger a Travis build.

> proposal: add nullability annotations to generated Java code
> 
>
> Key: THRIFT-4530
> URL: https://issues.apache.org/jira/browse/THRIFT-4530
> Project: Thrift
>  Issue Type: New Feature
>  Components: Java - Compiler
>Reporter: Manu Sridharan
>Priority: Major
>
> I'd like to propose (optionally) including {{@Nullable}} annotations in 
> Thrift-generated Java code.  I'm the main author of NullAway 
> ([https://github.com/uber/NullAway)] and we'd like to better support users 
> who are using Thrift.  The change would involve changing the Java code 
> generator to include {{@Nullable}} annotations on every field, method return 
> value, and method parameter in the public API of generated code that may be 
> null.  With these annotations, NullAway users will get warnings when their 
> client code is missing appropriate null checks.  Also, IDEs like IntelliJ 
> will give better warnings about missing null checks.  As part of this change, 
> I would also add support to NullAway for understanding {{isSetX()}} methods 
> to avoid excessive false positives.
> Regarding which {{@Nullable}} annotation to use, Thrift seems to try to 
> minimize third-party dependencies, but we could simply include a new Thrift 
> {{@Nullable}} annotation, and it will work fine with NullAway and most other 
> tools.
> I have a WIP patch to generate these annotations, but I wanted to get 
> feedback from the maintainers before opening a PR.  We could of course make 
> the annotation generation optional and default it to being off, if desired.  
> Anyway, thoughts / feedback welcome.  Thanks!



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (THRIFT-4482) error making c++ library on windows 8.1 using cygwin

2018-03-23 Thread Michel Max (JIRA)

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

Michel Max commented on THRIFT-4482:


I did not solve the issue (or do any sort of workaround), but the project i 
worked on no longer uses thrift. As far as I'm concerned, this issue could be 
closed, since I no longer use thrift.

> error making c++ library on windows 8.1 using cygwin
> 
>
> Key: THRIFT-4482
> URL: https://issues.apache.org/jira/browse/THRIFT-4482
> Project: Thrift
>  Issue Type: Bug
>  Components: Build Process, C++ - Library
>Affects Versions: 0.11.0
> Environment: Windows 8.1, thrift 0.11.0
> cygwin64, boost 1_66, GNU make 4.2.1, g++ 6.4.0, automake 1.15.1, autoconf 
> 2.69
>Reporter: Michel Max
>Assignee: James E. King, III
>Priority: Trivial
>  Labels: build, windows
>
> src/thrift/transport/THttpServer.cpp: In member function 'virtual void 
> apache::thrift::transport::THttpServer::parseHeader(char*)':
> src/thrift/transport/THttpServer.cpp:50:74: error: 'strcasestr' was not 
> declared in this scope
>    #define THRIFT_strcasestr(haystack, needle) strcasestr(haystack, needle)
>   ^
> src/thrift/transport/THttpServer.cpp:62:9: note: in expansion of macro 
> 'THRIFT_strcasestr'
>  if (THRIFT_strcasestr(value, "chunked") != NULL) {
>  ^
> make[4]: *** [Makefile:1350: src/thrift/transport/THttpServer.lo] Error 1
>  
> This is a part of the console output, but it is the entire error. This 
> happened during the execution of make in the root directory.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] thrift pull request #1519: THRIFT-4214: map key treated as hex va...

2018-03-23 Thread thexeos
GitHub user thexeos opened a pull request:

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

THRIFT-4214: map key treated as hex value in JavaScript

All strings passed to [node-int64 
constructor](https://github.com/broofa/node-int64/blob/c1567475712cb1cfe100c96813c2a2a92e2b42ce/Int64.js#L49)
 are treated as hex strings. We need to convert all decimal strings to hex 
representation before passing them to the constructor.

We are making an assumption that no hex string will be passed to 
`writeI64()` without 0x prefix.


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

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

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

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


commit 8ef113fd814ce195d06ee9c935ff702d3053c2b5
Author: thexeos 
Date:   2018-03-23T22:06:16Z

THRIFT-4214: map key treated as hex value in JavaScript




---


[jira] [Commented] (THRIFT-4214) map key treated as hex value in JavaScript

2018-03-23 Thread ASF GitHub Bot (JIRA)

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

ASF GitHub Bot commented on THRIFT-4214:


GitHub user thexeos opened a pull request:

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

THRIFT-4214: map key treated as hex value in JavaScript

All strings passed to [node-int64 
constructor](https://github.com/broofa/node-int64/blob/c1567475712cb1cfe100c96813c2a2a92e2b42ce/Int64.js#L49)
 are treated as hex strings. We need to convert all decimal strings to hex 
representation before passing them to the constructor.

We are making an assumption that no hex string will be passed to 
`writeI64()` without 0x prefix.


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

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

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

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


commit 8ef113fd814ce195d06ee9c935ff702d3053c2b5
Author: thexeos 
Date:   2018-03-23T22:06:16Z

THRIFT-4214: map key treated as hex value in JavaScript




> map key treated as hex value in JavaScript
> -
>
> Key: THRIFT-4214
> URL: https://issues.apache.org/jira/browse/THRIFT-4214
> Project: Thrift
>  Issue Type: Bug
>  Components: JavaScript - Compiler, JavaScript - Library
>Affects Versions: 0.10.0
>Reporter: Teodor Atroshenko
>Priority: Critical
>  Labels: easyfix
>
> The service is defined with the following function:
> {code:javascript}
> map someFunctionName()
> {code}
> The --gen js:node code:
> {code:javascript}
> output.writeFieldBegin('success', Thrift.Type.MAP, 0);
> output.writeMapBegin(Thrift.Type.I64, Thrift.Type.DOUBLE, 
> Thrift.objectLength(this.success));
> for (var kiter18 in this.success)
> {
>   if (this.success.hasOwnProperty(kiter18))
>   {
> var viter19 = this.success[kiter18];
> output.writeI64(kiter18);
> output.writeDouble(viter19);
>   }
> }
> output.writeMapEnd();
> output.writeFieldEnd();
> {code}
> String {{kiter18}} is passed to {{output.writeI64}}. Only *TBinaryProtocol* 
> is affected by this. The function 
> [defenition|https://github.com/apache/thrift/blob/19baeefd8c38d62085891d7956349601f79448b3/lib/nodejs/lib/thrift/binary_protocol.js#L137]:
> {code:javascript}
> TBinaryProtocol.prototype.writeI64 = function(i64) {
>   if (i64.buffer) {
> this.trans.write(i64.buffer);
>   } else {
> this.trans.write(new Int64(i64).buffer);
>   }
> };
> {code}
> *Int64* 
> [constructor|https://github.com/broofa/node-int64/blob/master/Int64.js#L49] 
> accepts the following arguments:
>  * new Int64(buffer\[, offset=0]) - Existing Buffer with byte offset
>  * new Int64(Uint8Array\[, offset=0]) - Existing Uint8Array with a byte offset
>  * new Int64(string) - Hex string (throws if n is outside int64 
> range)
>  * new Int64(number) - Number (throws if n is outside int64 range)
>  * new Int64(hi, lo) - Raw bits as two 32-bit values
> What happens, is that JavaScript object keys are always Strings, so the 
> generated function passes the object key as-is (in String representation) and 
> *Int64* library assumes it's a hex string and converts it to a different 
> 64-bit integer.
> For example {{"4398046511580"}} becomes {{\[Int64 value:1189123005158784 
> octets:00 04 39 80 46 51 15 80]}}
> This also happens when returning {{list}}, however in lists it can be 
> bypassed by calling {{Number()}} on all Array elements before passing it to 
> Thrift's {{result()}} function.
> To solve this, the compiler can be adding {{Number()}} around everything that 
> is a decimal number that is passed to {{writeI64}}; and if {{writeI64}} will 
> never be called with an actual hex string, then js library can be updated to 
> include the {{Number()}} call around *Int64* constructor argument.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (THRIFT-4530) proposal: add nullability annotations to generated Java code

2018-03-23 Thread Manu Sridharan (JIRA)

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

Manu Sridharan commented on THRIFT-4530:


Great!  I will definitely post the PR on GitHub. Do we want {{@Nullable}} 
annotations off by default, with a compiler flag to enable them?  Or should 
they just be on by default?  In any case, hope to have a PR up soon.  Thanks!

> proposal: add nullability annotations to generated Java code
> 
>
> Key: THRIFT-4530
> URL: https://issues.apache.org/jira/browse/THRIFT-4530
> Project: Thrift
>  Issue Type: New Feature
>  Components: Java - Compiler
>Reporter: Manu Sridharan
>Priority: Major
>
> I'd like to propose (optionally) including {{@Nullable}} annotations in 
> Thrift-generated Java code.  I'm the main author of NullAway 
> ([https://github.com/uber/NullAway)] and we'd like to better support users 
> who are using Thrift.  The change would involve changing the Java code 
> generator to include {{@Nullable}} annotations on every field, method return 
> value, and method parameter in the public API of generated code that may be 
> null.  With these annotations, NullAway users will get warnings when their 
> client code is missing appropriate null checks.  Also, IDEs like IntelliJ 
> will give better warnings about missing null checks.  As part of this change, 
> I would also add support to NullAway for understanding {{isSetX()}} methods 
> to avoid excessive false positives.
> Regarding which {{@Nullable}} annotation to use, Thrift seems to try to 
> minimize third-party dependencies, but we could simply include a new Thrift 
> {{@Nullable}} annotation, and it will work fine with NullAway and most other 
> tools.
> I have a WIP patch to generate these annotations, but I wanted to get 
> feedback from the maintainers before opening a PR.  We could of course make 
> the annotation generation optional and default it to being off, if desired.  
> Anyway, thoughts / feedback welcome.  Thanks!



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)