date:20170115

[GitHub] thrift issue #1156: THRIFT-4011 Use slices for Thrift sets

2017-01-15 Thread dcelasun

Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1156
  
> Good intentions, absolutely, but the implementation ..

FWIW, I agree. However, we still need a solution and as I explained above, 
we don't have a way of returning an error here. That leaves us with silent 
deduplication (during serialization) in lib/go/thrift. Are you OK with that?


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

[jira] [Commented] (THRIFT-4011) Sets of Thrift structs generate Go code that can't be serialized to JSON

2017-01-15 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on THRIFT-4011:


Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1156
  
> Good intentions, absolutely, but the implementation ..

FWIW, I agree. However, we still need a solution and as I explained above, 
we don't have a way of returning an error here. That leaves us with silent 
deduplication (during serialization) in lib/go/thrift. Are you OK with that?


> Sets of Thrift structs generate Go code that can't be serialized to JSON
> 
>
> Key: THRIFT-4011
> URL: https://issues.apache.org/jira/browse/THRIFT-4011
> Project: Thrift
>  Issue Type: Bug
>  Components: Go - Compiler
>Reporter: Can Celasun
>
> Consider the following structs:
> {code}
> struct Foo {
>   1: optional string foo
> }
> struct Bar {
>   1: optional set foos
> }
> {code}
> This compiles into the following Go code:
> {code}
> type Bar struct {
>   Foos map[*Foo]struct{} `thrift:"foos,1" db:"foos" json:"foos,omitempty"`
> }
> {code}
> Even though the generated code has tags for JSON support, Bar can't be 
> serialized to JSON:
> {code}
> json: unsupported type: map[*Foo]struct {}
> {code}
> One solution would be to use slices, not maps, for Thrift sets. Thoughts?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (THRIFT-4011) Sets of Thrift structs generate Go code that can't be serialized to JSON

2017-01-15 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on THRIFT-4011:


Github user Jens-G commented on the issue:

https://github.com/apache/thrift/pull/1156
  
Oopsie. After I read the word "map" above I must have been mentally 
switched to maps somehow.  My mistake.

Nevertheless what I said about panic and errors in general above still 
holds true. At this time we have exactly 0 (zero) panics in the code under 
/lib/go/thrift. So I would argue that introducing a panic at least breaks the 
usual patterns of that library.

>  Speaking of which, panicking in case of programmer error is very common 
and idiomatic in Go. The standard library is full of such panics 

Put four Go experts together and you get five opinions, all idiomatic. 
Especially about the high and lofty Go error handling design principles:

> In Go, error handling is important. **The language's design and 
conventions encourage you to explicitly check for errors** where they occur (as 
distinct from the convention in other languages of throwing exceptions and 
sometimes catching them). 

Or mostly assigning them to `_` as many people do in Go. That's for the 
"encourage you to explicitly check for errors" part. In contrast, an exception 
must be caught, not only "sometimes", because there is no other error handling 
option. 

> The decision to not include exceptions in Go is an example of its 
**simplicity** and orthogonality. Using multiple return values and a simple 
convention, **Go solves the problem** of letting programmers know when things 
have gone wrong and **reserves panic for the truly exceptional**.

I can't see any "simplicity" in replacing exceptions by panics, 
"encouraging" error handling by leading people into misusing the `_` operator 
and at the same time claiming "*this time, we get it right*". Go's error 
handling is FUBAR from the beginning to the end. Good intentions, absolutely, 
but the implementation  All personal opinion, of course, 




> Sets of Thrift structs generate Go code that can't be serialized to JSON
> 
>
> Key: THRIFT-4011
> URL: https://issues.apache.org/jira/browse/THRIFT-4011
> Project: Thrift
>  Issue Type: Bug
>  Components: Go - Compiler
>Reporter: Can Celasun
>
> Consider the following structs:
> {code}
> struct Foo {
>   1: optional string foo
> }
> struct Bar {
>   1: optional set foos
> }
> {code}
> This compiles into the following Go code:
> {code}
> type Bar struct {
>   Foos map[*Foo]struct{} `thrift:"foos,1" db:"foos" json:"foos,omitempty"`
> }
> {code}
> Even though the generated code has tags for JSON support, Bar can't be 
> serialized to JSON:
> {code}
> json: unsupported type: map[*Foo]struct {}
> {code}
> One solution would be to use slices, not maps, for Thrift sets. Thoughts?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] thrift issue #1156: THRIFT-4011 Use slices for Thrift sets

2017-01-15 Thread Jens-G

Github user Jens-G commented on the issue:

https://github.com/apache/thrift/pull/1156
  
Oopsie. After I read the word "map" above I must have been mentally 
switched to maps somehow.  My mistake.

Nevertheless what I said about panic and errors in general above still 
holds true. At this time we have exactly 0 (zero) panics in the code under 
/lib/go/thrift. So I would argue that introducing a panic at least breaks the 
usual patterns of that library.

>  Speaking of which, panicking in case of programmer error is very common 
and idiomatic in Go. The standard library is full of such panics 

Put four Go experts together and you get five opinions, all idiomatic. 
Especially about the high and lofty Go error handling design principles:

> In Go, error handling is important. **The language's design and 
conventions encourage you to explicitly check for errors** where they occur (as 
distinct from the convention in other languages of throwing exceptions and 
sometimes catching them). 

Or mostly assigning them to `_` as many people do in Go. That's for the 
"encourage you to explicitly check for errors" part. In contrast, an exception 
must be caught, not only "sometimes", because there is no other error handling 
option. 

> The decision to not include exceptions in Go is an example of its 
**simplicity** and orthogonality. Using multiple return values and a simple 
convention, **Go solves the problem** of letting programmers know when things 
have gone wrong and **reserves panic for the truly exceptional**.

I can't see any "simplicity" in replacing exceptions by panics, 
"encouraging" error handling by leading people into misusing the `_` operator 
and at the same time claiming "*this time, we get it right*". Go's error 
handling is FUBAR from the beginning to the end. Good intentions, absolutely, 
but the implementation  All personal opinion, of course, 




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

[GitHub] thrift issue #1088: Microsoft .Net Core library port and generator for this ...

2017-01-15 Thread vgotra

Github user vgotra commented on the issue:

https://github.com/apache/thrift/pull/1088
  
Hi @Jens-G 

Done - you can check master in https://github.com/vgotra/thrift.

Added framed transport and added sample of usage of MultiplexedProtocol. 
Also updated docs for library.

I matched that last time you used 1.0.0-preview2-1-003177 version of .Net 
Core SDK - you can also experiment with different SDK of .Net Core (just change 
version of SDK in solution global.json - I added samples). 

The only thing - in 1.0.0-preview2-1-003177 version of .NET Core CLI, 
dotnet doesn't support simple passing of arguments. So, for correct work of 
tutorial, arguments should be passed to cmd for server and client in full form 
like a (dotnet run -t:tcp -p:multiplexed, dotnet run -help). In latest SDK of 
.NET Core CLI added possibility to use passing arguments in more easy way - 
like: dotnet run -- -p:multiplexed, dotnet run -- -h

Pls, ping me when you will be ready to merge pull request and I will create 
it.


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

[jira] [Commented] (THRIFT-4029) Accelerated protocols do not build from thrift-py 0.10.0 on PyPI

2017-01-15 Thread Chandler May (JIRA)


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

Chandler May commented on THRIFT-4029:
--

The new zip distribution works for me.

> Accelerated protocols do not build from thrift-py 0.10.0 on PyPI
> 
>
> Key: THRIFT-4029
> URL: https://issues.apache.org/jira/browse/THRIFT-4029
> Project: Thrift
>  Issue Type: Bug
>  Components: Python - Library
>Affects Versions: 0.10.0
>Reporter: Chandler May
>Assignee: Jake Farrell
>
> The thrift 0.10.0 distribution on PyPI does not include extension headers and 
> a C++ (template) file, preventing the accelerated protocols from being built. 
>  {{pip install}} reports a brief error:
> {code}
> Running thrift-0.10.0/setup.py -q bdist_egg --dist-dir 
> /tmp/easy_install-bVU8VN/thrift-0.10.0/egg-dist-tmp-ghJzGL
> cc1plus: warning: command line option ‘-Wstrict-prototypes’ is valid for 
> C/ObjC but not for C++
> src/ext/module.cpp:21:19: fatal error: types.h: No such file or directory
>  #include "types.h"
>^
> compilation terminated.
> ()
> 
> An error occurred while trying to compile with the C extension enabled
> Attempting to build without the extension now
> 
> ()
> {code}
> The list of files that is not included in the distribution is as follows.  In 
> addition to the headers and source file mentioned above, there's a Windows 
> compatibility header missing and a couple of test files missing.  It looks 
> like there was a file extension filter (accidentally) applied to the C++ 
> files at least:
> {code}
> src/ext/binary.h
> src/ext/compact.h
> src/ext/endian.h
> src/ext/protocol.h
> src/ext/protocol.tcc
> src/ext/types.h
> compat/win32/stdint.h
> test/_import_local_thrift.py
> test/thrift_json.py
> {code}
> [~jfarrell] were you who made the release?  Is it possible to hotfix?  My 
> team has been waiting on the release of the accelerated compact protocol for 
> a while, this discovery is saddening for us.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (THRIFT-4011) Sets of Thrift structs generate Go code that can't be serialized to JSON

2017-01-15 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on THRIFT-4011:


Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1156
  
Hey @Jens-G, I'm not sure I follow you, what does this have anything to do 
with maps? Assuming you meant sets, the docs say:

> An unordered set of unique elements. Translates to an STL set, Java 
HashSet, set in Python, etc. Note: PHP does not support sets, so it is treated 
similar to a List

Similar to PHP, Go does not have a native type for sets, so the best thing 
to do is to treat it similar to a list.

> Given that, I would say it could be one option to error, when the user 
inserts a duplicate.

This is not possible, because the caller doesn't "insert" anything, they 
simply return a slice. Consider the following:

```thrift
service Foo {
set bar() throws (1: Something error)
}
```

This generates an interface called `Foo` with the following method:

```go
type Foo interface {
Bar() ([]string, error)
}
```

So the user simply returns a string slice, the Thrift library has no 
control over it. Once `Foo` returns, it's too late for Thrift itself to return 
an error, only panic. Speaking of which, panicking in case of ***programmer 
error*** is very common and idiomatic in Go. The standard library is full of 
such panics (e.g search for "misuse" 
[here](https://golang.org/src/sync/waitgroup.go)). I would consider returning a 
non-unique slice for a Thrift set a programming error (and hence deserving a 
panic), but if you disagree, I can update the PR with deduplication in the 
library.


> Sets of Thrift structs generate Go code that can't be serialized to JSON
> 
>
> Key: THRIFT-4011
> URL: https://issues.apache.org/jira/browse/THRIFT-4011
> Project: Thrift
>  Issue Type: Bug
>  Components: Go - Compiler
>Reporter: Can Celasun
>
> Consider the following structs:
> {code}
> struct Foo {
>   1: optional string foo
> }
> struct Bar {
>   1: optional set foos
> }
> {code}
> This compiles into the following Go code:
> {code}
> type Bar struct {
>   Foos map[*Foo]struct{} `thrift:"foos,1" db:"foos" json:"foos,omitempty"`
> }
> {code}
> Even though the generated code has tags for JSON support, Bar can't be 
> serialized to JSON:
> {code}
> json: unsupported type: map[*Foo]struct {}
> {code}
> One solution would be to use slices, not maps, for Thrift sets. Thoughts?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] thrift issue #1156: THRIFT-4011 Use slices for Thrift sets

2017-01-15 Thread dcelasun

Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1156
  
Hey @Jens-G, I'm not sure I follow you, what does this have anything to do 
with maps? Assuming you meant sets, the docs say:

> An unordered set of unique elements. Translates to an STL set, Java 
HashSet, set in Python, etc. Note: PHP does not support sets, so it is treated 
similar to a List

Similar to PHP, Go does not have a native type for sets, so the best thing 
to do is to treat it similar to a list.

> Given that, I would say it could be one option to error, when the user 
inserts a duplicate.

This is not possible, because the caller doesn't "insert" anything, they 
simply return a slice. Consider the following:

```thrift
service Foo {
set bar() throws (1: Something error)
}
```

This generates an interface called `Foo` with the following method:

```go
type Foo interface {
Bar() ([]string, error)
}
```

So the user simply returns a string slice, the Thrift library has no 
control over it. Once `Foo` returns, it's too late for Thrift itself to return 
an error, only panic. Speaking of which, panicking in case of ***programmer 
error*** is very common and idiomatic in Go. The standard library is full of 
such panics (e.g search for "misuse" 
[here](https://golang.org/src/sync/waitgroup.go)). I would consider returning a 
non-unique slice for a Thrift set a programming error (and hence deserving a 
panic), but if you disagree, I can update the PR with deduplication in the 
library.


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

[GitHub] thrift issue #1088: Microsoft .Net Core library port and generator for this ...

2017-01-15 Thread Jens-G

Github user Jens-G commented on the issue:

https://github.com/apache/thrift/pull/1088
  
Could you extract the other stuff (e.g. framed transport, multiplex, ...) 
into a separate PR that works with the current Thrift code base? Smaller PRs 
are easier to review, and we get at least these changes in early.


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

[jira] [Commented] (THRIFT-4011) Sets of Thrift structs generate Go code that can't be serialized to JSON

2017-01-15 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on THRIFT-4011:


Github user Jens-G commented on the issue:

https://github.com/apache/thrift/pull/1156
  
http://thrift.apache.org/docs/types states that (as one would expect) a 
Thrift map is defined as "A map of **strictly unique keys** to values. 
Translates to an STL map, Java HashMap, PHP associative array, Python/Ruby 
dictionary, etc.". 

Given that, I would say it could be one option to error, when the user 
inserts a duplicate. On the other hand, in that case the better option could be 
to simply replace the current value. 

But **what should not happen is that serialized data come in with 
duplicated keys**. That would be clearly an error as it is a violation of the 
rule above. In that case it might be absolutely ok to return some kind of error.

Re `panic()`, from my understanding of Go that's not the idiomatic way to 
go, since everything form tha above is an easily recoverable error.

Further reading:
 * https://blog.golang.org/error-handling-and-go
 * https://golang.org/doc/effective_go.html#panic
 * 
http://stackoverflow.com/questions/28472922/when-to-use-os-exit-and-panic-in-golang



> Sets of Thrift structs generate Go code that can't be serialized to JSON
> 
>
> Key: THRIFT-4011
> URL: https://issues.apache.org/jira/browse/THRIFT-4011
> Project: Thrift
>  Issue Type: Bug
>  Components: Go - Compiler
>Reporter: Can Celasun
>
> Consider the following structs:
> {code}
> struct Foo {
>   1: optional string foo
> }
> struct Bar {
>   1: optional set foos
> }
> {code}
> This compiles into the following Go code:
> {code}
> type Bar struct {
>   Foos map[*Foo]struct{} `thrift:"foos,1" db:"foos" json:"foos,omitempty"`
> }
> {code}
> Even though the generated code has tags for JSON support, Bar can't be 
> serialized to JSON:
> {code}
> json: unsupported type: map[*Foo]struct {}
> {code}
> One solution would be to use slices, not maps, for Thrift sets. Thoughts?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] thrift issue #1156: THRIFT-4011 Use slices for Thrift sets

2017-01-15 Thread Jens-G

Github user Jens-G commented on the issue:

https://github.com/apache/thrift/pull/1156
  
http://thrift.apache.org/docs/types states that (as one would expect) a 
Thrift map is defined as "A map of **strictly unique keys** to values. 
Translates to an STL map, Java HashMap, PHP associative array, Python/Ruby 
dictionary, etc.". 

Given that, I would say it could be one option to error, when the user 
inserts a duplicate. On the other hand, in that case the better option could be 
to simply replace the current value. 

But **what should not happen is that serialized data come in with 
duplicated keys**. That would be clearly an error as it is a violation of the 
rule above. In that case it might be absolutely ok to return some kind of error.

Re `panic()`, from my understanding of Go that's not the idiomatic way to 
go, since everything form tha above is an easily recoverable error.

Further reading:
 * https://blog.golang.org/error-handling-and-go
 * https://golang.org/doc/effective_go.html#panic
 * 
http://stackoverflow.com/questions/28472922/when-to-use-os-exit-and-panic-in-golang



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

[GitHub] thrift pull request #1153: fix a bug in windows

2017-01-15 Thread Jens-G

Github user Jens-G commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1153#discussion_r96138653
  
--- Diff: lib/cpp/src/thrift/server/TNonblockingServer.cpp ---
@@ -1324,7 +1324,7 @@ TNonblockingIOThread::~TNonblockingIOThread() {
 ownEventBase_ = false;
   }
 
-  if (listenSocket_ >= 0) {
+  if (listenSocket_ >= 0 && listenSocket_ != THRIFT_INVALID_SOCKET) {
--- End diff --

@GZShi : would you mind updating your PR accordingly?


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

[jira] [Commented] (THRIFT-2945) Implement support for Rust language

2017-01-15 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on THRIFT-2945:


Github user Jens-G commented on the issue:

https://github.com/apache/thrift/pull/1147
  
@all: Everyone happy with it?


> Implement support for Rust language
> ---
>
> Key: THRIFT-2945
> URL: https://issues.apache.org/jira/browse/THRIFT-2945
> Project: Thrift
>  Issue Type: New Feature
>  Components: Wish List
>Reporter: Maksim Golov
>Assignee: Jeremy Fitzhardinge
>
> Work on implementing support for Rust is in progress: 
> https://github.com/maximg/thrift by Simon Génier and myself.
> It will probably take quite some time to complete. Please keep us updated if 
> there are changes related to our work.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] thrift issue #1147: THRIFT-2945 Add Rust support

2017-01-15 Thread Jens-G

Github user Jens-G commented on the issue:

https://github.com/apache/thrift/pull/1147
  
@all: Everyone happy with it?


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

[jira] [Commented] (THRIFT-4011) Sets of Thrift structs generate Go code that can't be serialized to JSON

2017-01-15 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on THRIFT-4011:


GitHub user dcelasun opened a pull request:

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

THRIFT-4011 Use slices for Thrift sets

As discussed in 
[THRIFT-4011](https://issues.apache.org/jira/browse/THRIFT-4011), this commit 
changes the Go generator to use slices, instead of maps for Thrift sets.

I've specifically didn't touch the Go library since there was no agreement 
on panicking for duplicates. We have three options:

1. Leave it as is and add documentation stating deduplication is the 
caller's responsibility.
2. Silently deduplicate before serialization.
3. panic on duplicates.

2 and 3 probably requires 
[`reflect.DeepEqual`](https://golang.org/pkg/reflect/#DeepEqual), which is not 
ideal.

@Jens-G Thoughts?

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

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

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

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


commit 0e2e8c0b041300dafff641e19848a1e46df32bc6
Author: D. Can Celasun 
Date:   2017-01-15T09:53:19Z

THRIFT-4011 Use slices for Thrift sets

This commit changes the Go generator to use slices, instead of maps for 
Thrift sets.




> Sets of Thrift structs generate Go code that can't be serialized to JSON
> 
>
> Key: THRIFT-4011
> URL: https://issues.apache.org/jira/browse/THRIFT-4011
> Project: Thrift
>  Issue Type: Bug
>  Components: Go - Compiler
>Reporter: Can Celasun
>
> Consider the following structs:
> {code}
> struct Foo {
>   1: optional string foo
> }
> struct Bar {
>   1: optional set foos
> }
> {code}
> This compiles into the following Go code:
> {code}
> type Bar struct {
>   Foos map[*Foo]struct{} `thrift:"foos,1" db:"foos" json:"foos,omitempty"`
> }
> {code}
> Even though the generated code has tags for JSON support, Bar can't be 
> serialized to JSON:
> {code}
> json: unsupported type: map[*Foo]struct {}
> {code}
> One solution would be to use slices, not maps, for Thrift sets. Thoughts?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] thrift pull request #1156: THRIFT-4011 Use slices for Thrift sets

2017-01-15 Thread dcelasun

GitHub user dcelasun opened a pull request:

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

THRIFT-4011 Use slices for Thrift sets

As discussed in 
[THRIFT-4011](https://issues.apache.org/jira/browse/THRIFT-4011), this commit 
changes the Go generator to use slices, instead of maps for Thrift sets.

I've specifically didn't touch the Go library since there was no agreement 
on panicking for duplicates. We have three options:

1. Leave it as is and add documentation stating deduplication is the 
caller's responsibility.
2. Silently deduplicate before serialization.
3. panic on duplicates.

2 and 3 probably requires 
[`reflect.DeepEqual`](https://golang.org/pkg/reflect/#DeepEqual), which is not 
ideal.

@Jens-G Thoughts?

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

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

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

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


commit 0e2e8c0b041300dafff641e19848a1e46df32bc6
Author: D. Can Celasun 
Date:   2017-01-15T09:53:19Z

THRIFT-4011 Use slices for Thrift sets

This commit changes the Go generator to use slices, instead of maps for 
Thrift sets.




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

[GitHub] thrift issue #1156: THRIFT-4011 Use slices for Thrift sets

[jira] [Commented] (THRIFT-4011) Sets of Thrift structs generate Go code that can't be serialized to JSON

[jira] [Commented] (THRIFT-4011) Sets of Thrift structs generate Go code that can't be serialized to JSON

[GitHub] thrift issue #1156: THRIFT-4011 Use slices for Thrift sets

[GitHub] thrift issue #1088: Microsoft .Net Core library port and generator for this ...

[jira] [Commented] (THRIFT-4029) Accelerated protocols do not build from thrift-py 0.10.0 on PyPI

[jira] [Commented] (THRIFT-4011) Sets of Thrift structs generate Go code that can't be serialized to JSON

[GitHub] thrift issue #1156: THRIFT-4011 Use slices for Thrift sets

[GitHub] thrift issue #1088: Microsoft .Net Core library port and generator for this ...

[jira] [Commented] (THRIFT-4011) Sets of Thrift structs generate Go code that can't be serialized to JSON

[GitHub] thrift issue #1156: THRIFT-4011 Use slices for Thrift sets

[GitHub] thrift pull request #1153: fix a bug in windows

[jira] [Commented] (THRIFT-2945) Implement support for Rust language

[GitHub] thrift issue #1147: THRIFT-2945 Add Rust support

[jira] [Commented] (THRIFT-4011) Sets of Thrift structs generate Go code that can't be serialized to JSON

[GitHub] thrift pull request #1156: THRIFT-4011 Use slices for Thrift sets

16 matches

Site Navigation

Mail list logo

Footer information