[jira] [Commented] (THRIFT-4497) Erlang records should use map() for map type

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4497:


GitHub user dhull opened a pull request:

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

THRIFT-4497: Use map() field type for Erlang type for map struct fields

The Thrift Erlang code generator previously generated fields with the `#{}` 
Erlang type for maps fields.  In the Erlang type specification languages, 
however, `#{}` specifically means an empty map.  This commit fixes the code to 
emit `map()` instead, which means the maps keys and values may be of any type.

It would be possible to emit a field type such as `${keytype() => 
maptype()}`, but this commit does not do that.

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

$ git pull https://github.com/dhull/thrift thrift-4497-erlang-maps

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

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


commit 67f7f6343f71da75b8f2257a8bb63d2ebd48dc82
Author: David Hull 
Date:   2018-02-14T03:41:35Z

THRIFT-4497: Use `map()` field type for Erlang type for map struct fields.

The Thrift Erlang code generator previously generated fields with the
`#{}` Erlang type for maps fields.  In the Erlang type specification
languages, however, `#{}` specifically means an empty map.  This commit
fixes the code to emit `map()` instead, which means the maps keys and
values may be of any type.

It would be possible to emit a field type such as
`${keytype() => maptype()}`, but this commit does not do that.




> Erlang records should use map() for map type
> 
>
> Key: THRIFT-4497
> URL: https://issues.apache.org/jira/browse/THRIFT-4497
> Project: Thrift
>  Issue Type: Bug
>  Components: Erlang - Compiler
>Affects Versions: 0.11.0
>Reporter: David Hull
>Assignee: David Hull
>Priority: Major
>
> When the Thrift Erlang code generator is given the "maps" option, it 
> generates records with #{} as the field type. However #{} is the type for an 
> empty map. The Erlang [Types and Function 
> Specifications|http://erlang.org/doc/reference_manual/typespec.html#id79546] 
> says:
> {quote}
> Notice that the syntactic representation of {{map()}} is {{#\{any() => 
> any()\}}} (or {{#\{_ => _\}}}), not #\{ \}. The notation #\{ \} specifies the 
> singleton type for the empty map.
> {quote}



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

[GitHub] thrift pull request #1495: THRIFT-4497: Use map() field type for Erlang type...

[dhull]

GitHub user dhull opened a pull request:

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

THRIFT-4497: Use map() field type for Erlang type for map struct fields

The Thrift Erlang code generator previously generated fields with the `#{}` 
Erlang type for maps fields.  In the Erlang type specification languages, 
however, `#{}` specifically means an empty map.  This commit fixes the code to 
emit `map()` instead, which means the maps keys and values may be of any type.

It would be possible to emit a field type such as `${keytype() => 
maptype()}`, but this commit does not do that.

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

$ git pull https://github.com/dhull/thrift thrift-4497-erlang-maps

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

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


commit 67f7f6343f71da75b8f2257a8bb63d2ebd48dc82
Author: David Hull 
Date:   2018-02-14T03:41:35Z

THRIFT-4497: Use `map()` field type for Erlang type for map struct fields.

The Thrift Erlang code generator previously generated fields with the
`#{}` Erlang type for maps fields.  In the Erlang type specification
languages, however, `#{}` specifically means an empty map.  This commit
fixes the code to emit `map()` instead, which means the maps keys and
values may be of any type.

It would be possible to emit a field type such as
`${keytype() => maptype()}`, but this commit does not do that.




---

[jira] [Created] (THRIFT-4497) Erlang records should use map() for map type

[David Hull (JIRA)]

David Hull created THRIFT-4497:
--

 Summary: Erlang records should use map() for map type
 Key: THRIFT-4497
 URL: https://issues.apache.org/jira/browse/THRIFT-4497
 Project: Thrift
  Issue Type: Bug
  Components: Erlang - Compiler
Affects Versions: 0.11.0
Reporter: David Hull
Assignee: David Hull


When the Thrift Erlang code generator is given the "maps" option, it generates 
records with #{} as the field type. However #{} is the type for an empty map. 
The Erlang [Types and Function 
Specifications|http://erlang.org/doc/reference_manual/typespec.html#id79546] 
says:
{quote}
Notice that the syntactic representation of {{map()}} is {{#\{any() => 
any()\}}} (or {{#\{_ => _\}}}), not #\{ \}. The notation #\{ \} specifies the 
singleton type for the empty map.
{quote}




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

[jira] [Commented] (THRIFT-4460) php library use PSR-2

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4460:


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

https://github.com/apache/thrift/pull/1472#discussion_r168062703
  
--- Diff: build/docker/scripts/sca.sh ---
@@ -49,6 +49,10 @@ flake8 --ignore=E501 test/test.py
 flake8 --ignore=E501,E722 test/crossrunner
 flake8 test/features
 
+# PHP code style
+composer install --quiet
+./vendor/bin/phpcs
--- End diff --

@RobberPhex this change is causing errors in other builds even though I 
have rebased on master, see:

https://travis-ci.org/apache/thrift/jobs/341109387

I am going to disable it for now.


> php library use PSR-2
> -
>
> Key: THRIFT-4460
> URL: https://issues.apache.org/jira/browse/THRIFT-4460
> Project: Thrift
>  Issue Type: Improvement
>  Components: PHP - Library
>Affects Versions: 0.11.0
>Reporter: Robert Lu
>Assignee: Robert Lu
>Priority: Minor
> Fix For: 0.12.0
>
>
> PHP Library can use [PSR-2|http://www.php-fig.org/psr/psr-2/] as code standard



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

[GitHub] thrift pull request #1472: THRIFT-4460: PHP Library use PSR-2

[jeking3]

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

https://github.com/apache/thrift/pull/1472#discussion_r168062703
  
--- Diff: build/docker/scripts/sca.sh ---
@@ -49,6 +49,10 @@ flake8 --ignore=E501 test/test.py
 flake8 --ignore=E501,E722 test/crossrunner
 flake8 test/features
 
+# PHP code style
+composer install --quiet
+./vendor/bin/phpcs
--- End diff --

@RobberPhex this change is causing errors in other builds even though I 
have rebased on master, see:

https://travis-ci.org/apache/thrift/jobs/341109387

I am going to disable it for now.


---

[jira] [Commented] (THRIFT-4496) Screen keywords in service method names

[Jens Geyer (JIRA)]

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

Jens Geyer commented on THRIFT-4496:


If the idea is, however, to *get completely rid of the global list and put that 
buren onto the generators*, that is something I would *absolutely agree* to. I 
would also loudly vote for *creating an appropriate set of detailed test cases* 
before we even begin working on the feature.

 

> Screen keywords in service method names
> ---
>
> Key: THRIFT-4496
> URL: https://issues.apache.org/jira/browse/THRIFT-4496
> Project: Thrift
>  Issue Type: New Feature
>  Components: Python - Compiler
>Reporter: Vera Filippova
>Priority: Minor
>
> Apache Thrift compiler doesn't allow to use keywords in any of supported 
> languages as field names. However, there are other compilers, like Scrooge, 
> which do allow using some keywords as field identifiers, which leads to 
> incompatibility.
> Assume we had a service with 'delete' method, with Java code generated by 
> Scrooge. Now we'd like to generate Python code with Apache Thrift, but 
> encounter an error because of the 'delete' keyword.
> I understand that using only Apache Thrift compiler, a user will never 
> encounter this problem, but I think enabling keywords by request seems 
> feasible.
> h1. Proposal
> It's possible to tweak keywords on code generation stage, e.g. use 'delete_' 
> as a name of a generated function instead of 'delete', then use the original 
> method name for a protocol message: writeMethodBegin('delete').
> This feature could be enabled with an additional flag, e.g. --screen-keywords.
> I have a draft for python generator here [https://github.com/nsrtvwls/thrift]
> The questions are, is this functionality welcome? If yes, would it require to 
> have it supported for all languages?



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

[jira] [Commented] (THRIFT-4496) Screen keywords in service method names

[Jens Geyer (JIRA)]

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

Jens Geyer commented on THRIFT-4496:


In addition to the (by intention) not exhaustive global keyword list, there are 
solutions on a per language basis which make sure that proper code is generated 
that compiles. They merely work as you describe, by adding underscores or using 
[mechanisms like 
prefixes|https://msdn.microsoft.com/en-us/library/x53a06bb(v=vs.120).aspx] and 
if they don't work for a particular keyword then this is a problem that should 
be fixed. And there should not be an additional switch for it, because 
generating uncompileable code is a bug.

Second, the definition of "complete" is something that is hard to formalize, 
because it is context dependent. So the question stays, what context you plan 
to use for \{{--screen-keywords}} (and what "screen" means), i.e. who defines 
what a keyword is? To my understanding keywords are defined by the language 
designers, not by Scrooge.

The problem with a global, exhaustive list is furthermore, that different 
languages have different ideas about what a keyword is and what not. It is hard 
to explain why a particular name should be disallowed for everybody if only one 
out of 20 languages has a problem with it. The list today contains often used 
keywords across languages (and yes, there may be some exceptions to that rule 
of thumb for historical reasons)

Personal opinion: If we start to add "fixes" in Thrift to circumvent problems 
(or some vague "incompatibilities" that you mention) that other environments 
introduce ... well, I'm not sure if that is such a great idea.

 

> Screen keywords in service method names
> ---
>
> Key: THRIFT-4496
> URL: https://issues.apache.org/jira/browse/THRIFT-4496
> Project: Thrift
>  Issue Type: New Feature
>  Components: Python - Compiler
>Reporter: Vera Filippova
>Priority: Minor
>
> Apache Thrift compiler doesn't allow to use keywords in any of supported 
> languages as field names. However, there are other compilers, like Scrooge, 
> which do allow using some keywords as field identifiers, which leads to 
> incompatibility.
> Assume we had a service with 'delete' method, with Java code generated by 
> Scrooge. Now we'd like to generate Python code with Apache Thrift, but 
> encounter an error because of the 'delete' keyword.
> I understand that using only Apache Thrift compiler, a user will never 
> encounter this problem, but I think enabling keywords by request seems 
> feasible.
> h1. Proposal
> It's possible to tweak keywords on code generation stage, e.g. use 'delete_' 
> as a name of a generated function instead of 'delete', then use the original 
> method name for a protocol message: writeMethodBegin('delete').
> This feature could be enabled with an additional flag, e.g. --screen-keywords.
> I have a draft for python generator here [https://github.com/nsrtvwls/thrift]
> The questions are, is this functionality welcome? If yes, would it require to 
> have it supported for all languages?



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

[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=16363189#comment-16363189
 ] 

ASF GitHub Bot commented on THRIFT-4448:


Github user johnboiles commented on the issue:

https://github.com/apache/thrift/pull/1459
  
@jeking3 done


> 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
>Priority: Major
>
> 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
(v7.6.3#76005)

[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
  
@jeking3 done


---

[jira] [Commented] (THRIFT-4352) Ubuntu Artful doesn't appear to be compatible with Thrift and Haxe 3.4.2

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4352:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1465
  
I found a check-in to hxcpp around October 2017 which enabled C++11 by 
default; I had to disable it by setting this environment variable: `export 
HXCPP_CPP11=0` in the ubsan build script.


> Ubuntu Artful doesn't appear to be compatible with Thrift and Haxe 3.4.2
> 
>
> Key: THRIFT-4352
> URL: https://issues.apache.org/jira/browse/THRIFT-4352
> Project: Thrift
>  Issue Type: Bug
>  Components: Haxe - Library
>Affects Versions: 0.10.0
> Environment: docker ubuntu-artful with haxe 3.4.2 (stock)
>Reporter: James E. King, III
>Assignee: James E. King, III
>Priority: Major
>
> I'm working on the artful image, and haxe fails make check, so I have 
> disabled haxe in artful builds:
> {noformat}
> TestServerHandler.hx:51: testVoid()
> TestClient.hx:926: 1.262 ms per testVoid() call
> TestClient.hx:100: ===
> TestClient.hx:101: Tests executed122
> TestClient.hx:102: Tests succeeded   122 (100%)
> TestClient.hx:103: Tests failed  0 (0%)
> TestClient.hx:109: ===
> TestClient.hx:135: total test time: 1.396 seconds
> TSocket.hx:167: Eof Eof
> sleep 10
> timeout 20 php -S 127.0.0.1:9090 router.php &
> sleep 1
> PHP 7.1.8-1ubuntu1 Development Server started at Sun Oct  1 23:24:37 2017
> Listening on http://127.0.0.1:9090
> Document root is /thrift/src/test/haxe
> Press Ctrl-C to quit.
> bin/Main-debug client --transport http
> TestClient.hx:216: - http client : http://localhost:9090
> TestClient.hx:237: - binary protocol
> TestClient.hx:252: - 1 iterations
> TestClient.hx:437: testException("Xception")
> TestClient.hx:85: FAIL: testException("Xception")  -  OutsideBounds
> TestClient.hx:453: testException("TException")
> TestClient.hx:85: FAIL: testException("TException")  -  OutsideBounds
> TestClient.hx:473: testException("bla")
> TestClient.hx:85: FAIL: testException("bla")  -  OutsideBounds
> TestClient.hx:485: testVoid()
> Makefile:693: recipe for target 'check_php_web' failed
> make: *** [check_php_web] Segmentation fault (core dumped)
> {noformat}



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

[GitHub] thrift issue #1465: THRIFT-4352: update artful to use haxe 3.4.4 which fixes...

[jeking3]

Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1465
  
I found a check-in to hxcpp around October 2017 which enabled C++11 by 
default; I had to disable it by setting this environment variable: `export 
HXCPP_CPP11=0` in the ubsan build script.


---

[jira] [Created] (THRIFT-4496) Screen keywords in service method names

[Vera Filippova (JIRA)]

Vera Filippova created THRIFT-4496:
--

 Summary: Screen keywords in service method names
 Key: THRIFT-4496
 URL: https://issues.apache.org/jira/browse/THRIFT-4496
 Project: Thrift
  Issue Type: New Feature
  Components: Python - Compiler
Reporter: Vera Filippova


Apache Thrift compiler doesn't allow to use keywords in any of supported 
languages as field names. However, there are other compilers, like Scrooge, 
which do allow using some keywords as field identifiers, which leads to 
incompatibility.

Assume we had a service with 'delete' method, with Java code generated by 
Scrooge. Now we'd like to generate Python code with Apache Thrift, but 
encounter an error because of the 'delete' keyword.

I understand that using only Apache Thrift compiler, a user will never 
encounter this problem, but I think enabling keywords by request seems feasible.
h1. Proposal

It's possible to tweak keywords on code generation stage, e.g. use 'delete_' as 
a name of a generated function instead of 'delete', then use the original 
method name for a protocol message: writeMethodBegin('delete').

This feature could be enabled with an additional flag, e.g. --screen-keywords.

I have a draft for python generator here [https://github.com/nsrtvwls/thrift]

The questions are, is this functionality welcome? If yes, would it require to 
have it supported for all languages?



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

[jira] [Commented] (THRIFT-4495) Erlang records should allow 'undefined' for non-required fields

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4495:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1494
  
We use these versions in CI today, FYI:

trusty: R16B03
xenial: 18.3 (this is the one we run "make cross" under)
artful: 20.0.4



> Erlang records should allow 'undefined' for non-required fields
> ---
>
> Key: THRIFT-4495
> URL: https://issues.apache.org/jira/browse/THRIFT-4495
> Project: Thrift
>  Issue Type: Improvement
>  Components: Erlang - Compiler
>Affects Versions: 0.11.0
>Reporter: David Hull
>Assignee: David Hull
>Priority: Major
>
> The Erlang records created by the Erlang code generator allow only the type 
> declared by the Thrift definition file. If a field is not required, however, 
> the Erlang record should also allow the value {{undefined}} (this is similar 
> to a null value in other languages).
> Erlang includes a tool, dialyzer, that does type analysis of Erlang code. 
> Until Erlang 19, dialyzer implicitly added `undefined` as an allowed type for 
> all record fields, but as of Erlang 19 it no longer does. This means that 
> dialyzer now emits error messages whenever a record is constructed and 
> initial values are not specified for all of its fields.
> So, for example, the following thrift definition
> {noformat}
> struct Test {
>   1: required i32 a
>   2: i32 b
>   3: optional i32 c
> }{noformat}
> currently produced the following Erlang record:
> {noformat}
> -record('Test', {'a' :: integer(),
>  'b' :: integer(),
>  'c' :: integer()}).{noformat}
>  However it should produce the following:
> {noformat}
> -record('Test', {'a' :: integer(),
>  'b' :: integer() | undefined,
>  'c' :: integer() | undefined}).{noformat}



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

[GitHub] thrift issue #1494: THRIFT-4495: Allow `undefined` for non-required Erlang r...

[jeking3]

Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1494
  
We use these versions in CI today, FYI:

trusty: R16B03
xenial: 18.3 (this is the one we run "make cross" under)
artful: 20.0.4



---

[jira] [Commented] (THRIFT-4495) Erlang records should allow 'undefined' for non-required fields

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on THRIFT-4495:


GitHub user dhull opened a pull request:

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

THRIFT-4495: Allow `undefined` for non-required Erlang records fields.

As of Erlang 19, the dialyzer static type-analysis tool no longer 
implicitly adds `undefined` to the allowed types for a field.  This means that 
dialyzer will now complain about any non-required fields that are not 
explicitly initialed when creating a new record.

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

$ git pull https://github.com/dhull/thrift 
thrift-4495-erlang-undefined-fields

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

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


commit 332587370e89919fa34fcbab74d1bd99174d8e1a
Author: David Hull 
Date:   2018-02-13T01:11:24Z

THRIFT-4495: Allow `undefined` for non-required Erlang records fields.

As of Erlang 19, the dialyzer static type-analysis tool no longer
implicitly adds `undefined` to the allowed types for a field.  This
means that dialyzer will now complain about any non-required fields
that are not explicitly initialed when creating a new record.




> Erlang records should allow 'undefined' for non-required fields
> ---
>
> Key: THRIFT-4495
> URL: https://issues.apache.org/jira/browse/THRIFT-4495
> Project: Thrift
>  Issue Type: Improvement
>  Components: Erlang - Compiler
>Affects Versions: 0.11.0
>Reporter: David Hull
>Assignee: David Hull
>Priority: Major
>
> The Erlang records created by the Erlang code generator allow only the type 
> declared by the Thrift definition file. If a field is not required, however, 
> the Erlang record should also allow the value {{undefined}} (this is similar 
> to a null value in other languages).
> Erlang includes a tool, dialyzer, that does type analysis of Erlang code. 
> Until Erlang 19, dialyzer implicitly added `undefined` as an allowed type for 
> all record fields, but as of Erlang 19 it no longer does. This means that 
> dialyzer now emits error messages whenever a record is constructed and 
> initial values are not specified for all of its fields.
> So, for example, the following thrift definition
> {noformat}
> struct Test {
>   1: required i32 a
>   2: i32 b
>   3: optional i32 c
> }{noformat}
> currently produced the following Erlang record:
> {noformat}
> -record('Test', {'a' :: integer(),
>  'b' :: integer(),
>  'c' :: integer()}).{noformat}
>  However it should produce the following:
> {noformat}
> -record('Test', {'a' :: integer(),
>  'b' :: integer() | undefined,
>  'c' :: integer() | undefined}).{noformat}



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

[GitHub] thrift pull request #1494: THRIFT-4495: Allow `undefined` for non-required E...

[dhull]

GitHub user dhull opened a pull request:

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

THRIFT-4495: Allow `undefined` for non-required Erlang records fields.

As of Erlang 19, the dialyzer static type-analysis tool no longer 
implicitly adds `undefined` to the allowed types for a field.  This means that 
dialyzer will now complain about any non-required fields that are not 
explicitly initialed when creating a new record.

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

$ git pull https://github.com/dhull/thrift 
thrift-4495-erlang-undefined-fields

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

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


commit 332587370e89919fa34fcbab74d1bd99174d8e1a
Author: David Hull 
Date:   2018-02-13T01:11:24Z

THRIFT-4495: Allow `undefined` for non-required Erlang records fields.

As of Erlang 19, the dialyzer static type-analysis tool no longer
implicitly adds `undefined` to the allowed types for a field.  This
means that dialyzer will now complain about any non-required fields
that are not explicitly initialed when creating a new record.




---

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

[jeking3]

Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1459
  
Would you mind squashing this into a single commit?


---

[jira] [Created] (THRIFT-4495) Erlang records should allow 'undefined' for non-required fields

[David Hull (JIRA)]

David Hull created THRIFT-4495:
--

 Summary: Erlang records should allow 'undefined' for non-required 
fields
 Key: THRIFT-4495
 URL: https://issues.apache.org/jira/browse/THRIFT-4495
 Project: Thrift
  Issue Type: Improvement
  Components: Erlang - Compiler
Affects Versions: 0.11.0
Reporter: David Hull
Assignee: David Hull


The Erlang records created by the Erlang code generator allow only the type 
declared by the Thrift definition file. If a field is not required, however, 
the Erlang record should also allow the value {{undefined}} (this is similar to 
a null value in other languages).

Erlang includes a tool, dialyzer, that does type analysis of Erlang code. Until 
Erlang 19, dialyzer implicitly added `undefined` as an allowed type for all 
record fields, but as of Erlang 19 it no longer does. This means that dialyzer 
now emits error messages whenever a record is constructed and initial values 
are not specified for all of its fields.

So, for example, the following thrift definition
{noformat}
struct Test {
  1: required i32 a
  2: i32 b
  3: optional i32 c
}{noformat}
currently produced the following Erlang record:
{noformat}
-record('Test', {'a' :: integer(),
 'b' :: integer(),
 'c' :: integer()}).{noformat}
 However it should produce the following:
{noformat}
-record('Test', {'a' :: integer(),
 'b' :: integer() | undefined,
 'c' :: integer() | undefined}).{noformat}



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

[jira] [Commented] (THRIFT-4494) Increase Java Socket Buffer Size

[BELUGA BEHR (JIRA)]

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

BELUGA BEHR commented on THRIFT-4494:
-

Here, the buffer is hard-coded to 8192 (which matches the default value on most 
JDK implementations).  You may wish to remove this magic number and let the JDK 
decide the size.

https://github.com/apache/thrift/blob/19baeefd8c38d62085891d7956349601f79448b3/lib/java/src/org/apache/thrift/transport/TFileTransport.java#L369

> Increase Java Socket Buffer Size
> 
>
> Key: THRIFT-4494
> URL: https://issues.apache.org/jira/browse/THRIFT-4494
> Project: Thrift
>  Issue Type: Improvement
>  Components: Java - Library
>Affects Versions: 0.11.0
>Reporter: BELUGA BEHR
>Priority: Minor
>
> {code:title=TSocket.java}
>   if (isOpen()) {
>   try {
> inputStream_ = new BufferedInputStream(socket_.getInputStream(), 
> 1024);
> outputStream_ = new BufferedOutputStream(socket_.getOutputStream(), 
> 1024);
>   } catch (IOException iox) {
> close();
> throw new TTransportException(TTransportException.NOT_OPEN, iox);
>   }
> }
> {code}
> The 1024 buffer size is pretty narrow, especially for modern servers with 
> TCP/IP send and receive buffers ranging from 16K to 64K.  Please remove these 
> hard coded values and rely on the underlying JVM default buffer sizes: 8Kib 
> on most implementations.
> https://github.com/apache/thrift/blob/19baeefd8c38d62085891d7956349601f79448b3/lib/java/src/org/apache/thrift/transport/TSocket.java



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

[jira] [Created] (THRIFT-4494) Increase Java Socket Buffer Size

[BELUGA BEHR (JIRA)]

BELUGA BEHR created THRIFT-4494:
---

 Summary: Increase Java Socket Buffer Size
 Key: THRIFT-4494
 URL: https://issues.apache.org/jira/browse/THRIFT-4494
 Project: Thrift
  Issue Type: Improvement
  Components: Java - Library
Affects Versions: 0.11.0
Reporter: BELUGA BEHR


{code:title=TSocket.java}
  if (isOpen()) {
  try {
inputStream_ = new BufferedInputStream(socket_.getInputStream(), 1024);
outputStream_ = new BufferedOutputStream(socket_.getOutputStream(), 
1024);
  } catch (IOException iox) {
close();
throw new TTransportException(TTransportException.NOT_OPEN, iox);
  }
}
{code}

The 1024 buffer size is pretty narrow, especially for modern servers with 
TCP/IP send and receive buffers ranging from 16K to 64K.  Please remove these 
hard coded values and rely on the underlying JVM default buffer sizes: 8Kib on 
most implementations.

https://github.com/apache/thrift/blob/19baeefd8c38d62085891d7956349601f79448b3/lib/java/src/org/apache/thrift/transport/TSocket.java



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