[jira] [Commented] (THRIFT-1067) Tons of bugs in php implementation
[ https://issues.apache.org/jira/browse/THRIFT-1067?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13038020#comment-13038020 ] Jeff Whiting commented on THRIFT-1067: -- I think the php thrift extension version number should be incremented whenever a bug fix / change is made to the extension. Right now it is still at 1.0 but it should be 1.0.1 or 1.1 after applying this patch. That way when people have problems you can have them check their extension version and know if they have the latest one installed with all the bug fixes. Tons of bugs in php implementation -- Key: THRIFT-1067 URL: https://issues.apache.org/jira/browse/THRIFT-1067 Project: Thrift Issue Type: Bug Components: PHP - Library Affects Versions: 0.6 Reporter: ruslan.usifov Assignee: ruslan.usifov Fix For: 0.7 Attachments: php20110219.patch PHP version of thrift hold many bugs: 1). Incorrect work with ZTS 2). Hung when work throw thrift framed transport 3). Memleak in protocol_writeMessageBegin 4). Hung when server close connection, and when php code try reuse same connection 5). Impossible pass really long values on 32 bit platforms -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (THRIFT-1067) Tons of bugs in php implementation
[ https://issues.apache.org/jira/browse/THRIFT-1067?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13038030#comment-13038030 ] Jake Farrell commented on THRIFT-1067: -- Jeff, THRIFT-985 is open for versioning the php client Tons of bugs in php implementation -- Key: THRIFT-1067 URL: https://issues.apache.org/jira/browse/THRIFT-1067 Project: Thrift Issue Type: Bug Components: PHP - Library Affects Versions: 0.6 Reporter: ruslan.usifov Assignee: ruslan.usifov Fix For: 0.7 Attachments: php20110219.patch PHP version of thrift hold many bugs: 1). Incorrect work with ZTS 2). Hung when work throw thrift framed transport 3). Memleak in protocol_writeMessageBegin 4). Hung when server close connection, and when php code try reuse same connection 5). Impossible pass really long values on 32 bit platforms -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (THRIFT-1115) python TBase class for dynamic (de)serialization, and __slots__ option for memory savings
[
https://issues.apache.org/jira/browse/THRIFT-1115?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13038066#comment-13038066
]
David Reiss commented on THRIFT-1115:
-
Looks solid. Mostly style changes.
Does dynamic require the slots option and conflict with the newstyle option?
If so, this should be enforced. If not, there are a few places where they
conflict.
In TExceptionBase, can you copy the read and write methods the way you do with
__eq__?
{noformat}
+ except TypeError:
+pass # TODO: add logging about unhashable types. i.e. d=dict();
d[[0,1]] = 2 fails
{noformat}
This should be an exception.
{noformat}
@@ -176,7 +180,6 @@
except Xception, x:
self.assertEqual(x.errorCode, 1001)
self.assertEqual(x.message, 'Xception')
-
try:
self.client.testException(throw_undeclared)
self.fail(should have thrown exception)
@@ -225,4 +228,4 @@
self.createTests()
{noformat}
Looks like this is an unnecessary change.
{noformat}
+// hdr += std::string(from thrift.protocol.TBase import TBase\n);
{noformat}
Remove commented-out code.
{noformat}
+hdr +=
+ from thrift.transport import TTransport\n
+ from thrift.protocol import TBinaryProtocol, TProtocol\n;
+hdr += try:\n
from thrift.protocol import fastbinary\n
except:\n
fastbinary = None\n;
{noformat}
The two hdr += statements can be combined into one.
In fact, it looks like the hdr variable can be eliminated entirely if you want.
{noformat}
@@ -384,6 +439,7 @@
f_types_
class tenum-get_name()
(gen_newstyle_ ? (object) : )
+(gen_dynamic_ ? ( + gen_dynbaseclass_ + ) : )
: endl;
{noformat}
This is one conflict between newstyle and dynamic. I think the generation of
__eq__ and friends is another.
{noformat}
+if (gen_newstyle_) {
+ out (object);
+}
+else if (gen_dynamic_) {
+ out ( gen_dynbaseclass_ );
+}
{noformat}
Combine else if to previous line.
{noformat}
+indent_down();
+ } else {
+if (! gen_dynamic_) {
+ // no base class available to implement __eq__ and __repr__ and __ne__
for us
{noformat}
Combine else with if. Also, indentation is off inside the if.
{noformat}
+// perhaps we should try __slots__?
{noformat}
What does this mean?
{noformat}
+indent_up();
+for (m_iter = sorted_members.begin(); m_iter != sorted_members.end();
++m_iter) {
+ indent(out) ' (*m_iter)-get_name() ',
+ endl;
+}
{noformat}
This puts trailing whitespace in generated code.
{noformat}
+ if (! gen_dynamic_) {
{noformat}
No space after !.
{noformat}
+ indent()for attr in self.__slots__: endl
+ indent() my_val, other_val = [getattr(obj, attr) for obj in
[self, other]] endl
+ indent() if my_val is other_val: endl
+ indent()continue endl
+ indent() if my_val != other_val: endl
+ indent() return False endl
+ indent()return True endl
{noformat}
Please make the same updates as you did to the TBase implementation (two
separte statements to extract vals, and skip the is check).
{noformat}
if (gen_twisted_) {
extends_if = (Interface);
+} else if (gen_dynamic_) {
+ extends_if = ( + gen_dynbaseclass_ + ); // TBase
{noformat}
The service interface should not be a TBase.
{noformat}
+if (gen_twisted_ gen_newstyle_) { // TODO: handle dynbase here?
{noformat}
Same for the client.
python TBase class for dynamic (de)serialization, and __slots__ option for
memory savings
-
Key: THRIFT-1115
URL: https://issues.apache.org/jira/browse/THRIFT-1115
Project: Thrift
Issue Type: New Feature
Components: Build Process, Python - Compiler, Python - Library
Reporter: Will Pierce
Assignee: Will Pierce
Attachments: THRIFT-1115.python_dynamic_code_and_slots_v1.patch,
THRIFT-1115.python_dynamic_code_and_slots_v2.patch,
THRIFT-1115.python_dynamic_code_and_slots_v3.patch, test_dynser.py,
test_size.py
This patch adds several new features to the compiler for python and the
python libraries, and exercises the new features with expanded unit testing.
This adds support for generating python classes that have no {{read()}} or
{{write()}} methods. Instead, the generated classes inherit from a new base
class, {{TProtocolDynamic}}. This new class implements the de/serialization
with {{read()}} and {{write()}} methods that iterate over the derived class's
{{thrift_spec}} class member that describes the structure and types of the
thrift. This dynamic evaluation works with both binary and compact
protocols, and has the same hook as before for delegating de/serialization to
Re: Pluggable Serializers
I certainly wouldn't want to see the global interface design you proposed.
That would be awkward.
I was thinking something more along the lines of your factory idea, but
without hardcoding the serializer styles. Instead, it would be up to the
factory to select the appropriate serializer. Then it becomes an exercise in
classifying protocols so that it's clear which kind of serializer they
should use. For instance, Binary and (existing) Compact both use the
context free serializer; JSON would probably use a custom serializer, as
would a rewritten Compact.
I'm not sure how to go into too much more detail without writing code. Are
you in a position where you want to start hacking on this project? If so, we
can chat offline about how to get a prototype going.
-Bryan
On Sun, May 22, 2011 at 10:51 AM, David Nadlinger c...@klickverbot.atwrote:
Hey Bryan,
First, I'd like to thank you a lot for your offer – I very much appreciate
any help from more experienced Thrift users or developers.
I thought a bit more about this issue, and while I agree that the current
scheme makes it really hard to implement alternative protocols differing
from the flat, »context-free« nature of the default binary protocol, I'm not
sure how pluggable serializers would be implemented in your idea.
More specifically, I can't quite see how structs would really be serialized
after the change. Would you propose to replace the protocol interface by a
project-specific generated serializer interface having a write method for
all defined struct types, like in the following example?
---
struct Foo {
int a;
// No read/write method here.
}
struct Bar { … }
interface TSerializer {
void writeFoo(Foo f);
void writeBar(Bar b);
}
class TBinarySerializer implements TSerializer {
this (TTransport t) { … }
void writeFoo(Foo f) { … }
void writeBar(Bar b) { … }
}
class TJsonSerializer implements TSerializer { … }
---
Having such a single global interface doesn't seem quite right to me
(extensibility, etc.) even if it would be generated, and indeed you wrote
about serializer classes being generated for each struct. But how would you
connect serializers to protocols then, or how would the protocol interface
(i.e. TProtocol and friends) look like in the first place to allow for
writing protocol agnostic code? It appears to me that somewhere all possible
»protocol styles« (i.e. serializer types) would have to be enumerated,
because otherwise there would be no way for the write() methods to be able
to select the correct serializer, which doesn't seem like a great solution
either.
To clarify what I mean, another example how I think this approach could be
implemented:
---
interface TProtocol {
void writeStruct(TStructSerializerFactory s);
…
}
class TBinaryProtocol implements TProtocol {
void writeStruct(TStructSerializerFactory s) {
s.getBinarySerializer().writeTo(this);
}
…
}
interface TBinarySerializer { void writeTo(TBinaryProtocol t); }
interface TJsonSerializer { void writeTo(TJsonProtocol t); }
interface TStructSerializerFactory {
// Have to enumerate all possible protocol »styles« here.
TBinarySerializer getBinarySerializer();
TJsonSerializer getJsonSerializer();
…
}
struct Foo {
int a;
void write(TProtocol t) {
t.writeStruct(new FooSerializerFactory(this));
}
}
class FooSerializerFactory implements TStructSerializerFactory {
Foo f_;
this(Foo f) {
f_ = f;
}
TBinarySerializer getBinarySerializer() {
return new FooBinarySerializer(f_);
}
// other factory methods
…
}
class FooBinarySerializer implements TBinarySerializer {
Foo f_;
this(Foo f) {
f_ = f;
}
void writeTo(TBinaryProtocol t) {
// The code currently generated into Foo.write().
…
}
}
---
There are of course a few other possible ways to implement this, but I
couldn't really come up with a design to connect serializers and protocols
that doesn't seem hackish or overly complex.
But isn't the problem really just that the current TProtocol interface
makes it hard to implement protocols that have some kind of »scope« or
»nesting«, like JSON does, because everything is »flattened« to a single
layer, only to painstakingly reconstruct the structure from the
write*Begin() and write*End() calls later?
I think it would help quite a bit to just replace all the pairs of *Begin()
and *End() calls with a single function, e.g. writeStruct(), which takes a
delegate/lambda (or whatever it is called in the respective language) for
writing the children. A little piece of D-style pseudocode to illustrate
what I mean:
---
interface TProtocol {
void writeStruct(string name, void delegate() writeMembers);
…
}
class TJsonProtocol implements TProtocol {
void writeStruct(string name, void delegate() writeMembers) {
// Do some setup work, open a new JSON object.
…
// Call the passed
[jira] [Commented] (THRIFT-1177) Update thrift to reflect changes in Go's networking libraries
[ https://issues.apache.org/jira/browse/THRIFT-1177?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13038117#comment-13038117 ] Bryan Duxbury commented on THRIFT-1177: --- Anyone reading this feel like they can give this a review? Update thrift to reflect changes in Go's networking libraries - Key: THRIFT-1177 URL: https://issues.apache.org/jira/browse/THRIFT-1177 Project: Thrift Issue Type: Dependency upgrade Components: Go - Compiler, Go - Library Affects Versions: 0.7 Reporter: Aalok Shah Priority: Minor Fix For: 0.7 Attachments: gopatch2.diff Go updated its networking libraries requiring a change to both the thrift code generator and the libraries. Additionally, the Go generator did not support arguments/variables that were Go keywords, this needs to be updated to work with Cassandra's thrift since range is a Go keyword. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Closed] (THRIFT-1178) Java: TBase signature should be T extends TBase?,?
[
https://issues.apache.org/jira/browse/THRIFT-1178?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Bryan Duxbury closed THRIFT-1178.
-
Resolution: Fixed
Fix Version/s: 0.7
Assignee: ryan rawson
I just committed this to TRUNK.
Java: TBase signature should be T extends TBase?,?
Key: THRIFT-1178
URL: https://issues.apache.org/jira/browse/THRIFT-1178
Project: Thrift
Issue Type: Bug
Components: Java - Library
Affects Versions: 0.6.1
Reporter: ryan rawson
Assignee: ryan rawson
Fix For: 0.7
Attachments: THRIFT-1178.txt
Right now TBase looks like:
public interface TBaseT extends TBase, F extends TFieldIdEnum extends
ComparableT, Serializable {
but it should be:
public interface TBaseT extends TBase?,?, F extends TFieldIdEnum extends
ComparableT, Serializable {
While the Java compiler does not have a problem, the Scala compiler throws a
fit and refuses to upcast generated classes to 'TBase' when required, such as
calling TSerializer.serialize
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
Build failed in Jenkins: Thrift #146
See https://builds.apache.org/hudson/job/Thrift/146/changes Changes: [bryanduxbury] THRIFT-1178. java: Java: TBase signature should be T extends TBase?,? Patch: Ryan Rawson -- [...truncated 698 lines...] /bin/bash ../../libtool --tag=CXX --mode=link g++ -Wall -g -O2 -o libthriftz.la -rpath /usr/local/lib libthriftz_la-TZlibTransport.lo -lssl -lrt -lpthread libtool: link: g++ -shared -nostdlib /usr/lib/gcc/x86_64-linux-gnu/4.4.3/../../../../lib/crti.o /usr/lib/gcc/x86_64-linux-gnu/4.4.3/crtbeginS.o .libs/libthriftz_la-TZlibTransport.o -lssl -lrt -lpthread -L/usr/lib/gcc/x86_64-linux-gnu/4.4.3 -L/usr/lib/gcc/x86_64-linux-gnu/4.4.3/../../../../lib -L/lib/../lib -L/usr/lib/../lib -L/usr/lib/gcc/x86_64-linux-gnu/4.4.3/../../.. -lstdc++ -lm -lc -lgcc_s /usr/lib/gcc/x86_64-linux-gnu/4.4.3/crtendS.o /usr/lib/gcc/x86_64-linux-gnu/4.4.3/../../../../lib/crtn.o-Wl,-soname -Wl,libthriftz.so.0 -o .libs/libthriftz.so.0.0.0 libtool: link: (cd .libs rm -f libthriftz.so.0 ln -s libthriftz.so.0.0.0 libthriftz.so.0) libtool: link: (cd .libs rm -f libthriftz.so ln -s libthriftz.so.0.0.0 libthriftz.so) libtool: link: ar cru .libs/libthriftz.a libthriftz_la-TZlibTransport.o libtool: link: ranlib .libs/libthriftz.a libtool: link: ( cd .libs rm -f libthriftz.la ln -s ../libthriftz.la libthriftz.la ) g++ -DHAVE_CONFIG_H -I. -I../.. -I/usr/include -I./src -Wall -g -O2 -MT Tests.o -MD -MP -MF .deps/Tests.Tpo -c -o Tests.o `test -f 'src/concurrency/test/Tests.cpp' || echo './'`src/concurrency/test/Tests.cpp mv -f .deps/Tests.Tpo .deps/Tests.Po /bin/bash ../../libtool --tag=CXX --mode=link g++ -Wall -g -O2 -o concurrency_test Tests.o libthrift.la -lssl -lrt -lpthread libtool: link: g++ -Wall -g -O2 -o .libs/concurrency_test Tests.o ./.libs/libthrift.so -lssl -lrt -lpthread make[4]: Leaving directory `https://builds.apache.org/hudson/job/Thrift/ws/thrift/lib/cpp' Making all in test make[4]: Entering directory `https://builds.apache.org/hudson/job/Thrift/ws/thrift/lib/cpp/test' ../../../compiler/cpp/thrift --gen cpp:dense ../../../test/DebugProtoTest.thrift [WARNING:https://builds.apache.org/hudson/job/Thrift/ws/thrift/test/DebugProtoTest.thrift:41] 64-bit constant 100 may not work in all languages. [WARNING:https://builds.apache.org/hudson/job/Thrift/ws/thrift/test/DebugProtoTest.thrift:169] 64-bit constant 1099511627775 may not work in all languages. [WARNING:https://builds.apache.org/hudson/job/Thrift/ws/thrift/test/DebugProtoTest.thrift:179] 64-bit constant 4294967295 may not work in all languages. [WARNING:https://builds.apache.org/hudson/job/Thrift/ws/thrift/test/DebugProtoTest.thrift:179] 64-bit constant 1099511627775 may not work in all languages. [WARNING:https://builds.apache.org/hudson/job/Thrift/ws/thrift/test/DebugProtoTest.thrift:179] 64-bit constant 281474976710655 may not work in all languages. [WARNING:https://builds.apache.org/hudson/job/Thrift/ws/thrift/test/DebugProtoTest.thrift:179] 64-bit constant 72057594037927935 may not work in all languages. [WARNING:https://builds.apache.org/hudson/job/Thrift/ws/thrift/test/DebugProtoTest.thrift:179] 64-bit constant 9223372036854775807 may not work in all languages. [WARNING:https://builds.apache.org/hudson/job/Thrift/ws/thrift/test/DebugProtoTest.thrift:188] 64-bit constant 4294967295 may not work in all languages. [WARNING:https://builds.apache.org/hudson/job/Thrift/ws/thrift/test/DebugProtoTest.thrift:188] 64-bit constant 1099511627775 may not work in all languages. [WARNING:https://builds.apache.org/hudson/job/Thrift/ws/thrift/test/DebugProtoTest.thrift:188] 64-bit constant 281474976710655 may not work in all languages. [WARNING:https://builds.apache.org/hudson/job/Thrift/ws/thrift/test/DebugProtoTest.thrift:188] 64-bit constant 72057594037927935 may not work in all languages. [WARNING:https://builds.apache.org/hudson/job/Thrift/ws/thrift/test/DebugProtoTest.thrift:188] 64-bit constant 9223372036854775807 may not work in all languages. [WARNING:https://builds.apache.org/hudson/job/Thrift/ws/thrift/test/DebugProtoTest.thrift:197] 64-bit constant 9223372036854775807 may not work in all languages. [WARNING:https://builds.apache.org/hudson/job/Thrift/ws/thrift/test/DebugProtoTest.thrift:204] 64-bit constant 9223372036854775807 may not work in all languages. [WARNING:https://builds.apache.org/hudson/job/Thrift/ws/thrift/test/DebugProtoTest.thrift:41] 64-bit constant 100 may not work in all languages. [WARNING:https://builds.apache.org/hudson/job/Thrift/ws/thrift/test/DebugProtoTest.thrift:169] 64-bit constant 1099511627775 may not work in all languages. [WARNING:https://builds.apache.org/hudson/job/Thrift/ws/thrift/test/DebugProtoTest.thrift:179] 64-bit constant 4294967295 may not work in all languages. [WARNING:https://builds.apache.org/hudson/job/Thrift/ws/thrift/test/DebugProtoTest.thrift:179] 64-bit constant 1099511627775 may not
Jenkins build is back to normal : Thrift #147
See https://builds.apache.org/hudson/job/Thrift/147/changes
[jira] [Commented] (THRIFT-1178) Java: TBase signature should be T extends TBase?,?
[
https://issues.apache.org/jira/browse/THRIFT-1178?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13038273#comment-13038273
]
Hudson commented on THRIFT-1178:
Integrated in Thrift #147 (See
[https://builds.apache.org/hudson/job/Thrift/147/])
Java: TBase signature should be T extends TBase?,?
Key: THRIFT-1178
URL: https://issues.apache.org/jira/browse/THRIFT-1178
Project: Thrift
Issue Type: Bug
Components: Java - Library
Affects Versions: 0.6.1
Reporter: ryan rawson
Assignee: ryan rawson
Fix For: 0.7
Attachments: THRIFT-1178.txt
Right now TBase looks like:
public interface TBaseT extends TBase, F extends TFieldIdEnum extends
ComparableT, Serializable {
but it should be:
public interface TBaseT extends TBase?,?, F extends TFieldIdEnum extends
ComparableT, Serializable {
While the Java compiler does not have a problem, the Scala compiler throws a
fit and refuses to upcast generated classes to 'TBase' when required, such as
calling TSerializer.serialize
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (THRIFT-1170) Thrift Generated Code and Java 5
[ https://issues.apache.org/jira/browse/THRIFT-1170?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13038274#comment-13038274 ] Hudson commented on THRIFT-1170: Integrated in Thrift #147 (See [https://builds.apache.org/hudson/job/Thrift/147/]) THRIFT-1170. java: Thrift Generated Code and Java 5 This patch adds a trivial java5 option to the java generator. Patch: Greg Moulliet bryanduxbury : http://svn.apache.org/viewvc/?view=revrev=1126765 Files : * /thrift/trunk/compiler/cpp/src/generate/t_java_generator.cc * /thrift/trunk/lib/java/src/org/apache/thrift/TUnion.java Thrift Generated Code and Java 5 Key: THRIFT-1170 URL: https://issues.apache.org/jira/browse/THRIFT-1170 Project: Thrift Issue Type: Improvement Components: Java - Compiler Affects Versions: 0.6.1 Reporter: Greg Moulliet Assignee: Greg Moulliet Fix For: 0.7 Attachments: java5.patch, java5_trunk.patch The Thrift Java compiler creates code that is not compliant with Java 5. One issue is the @Override annotation on the clear() method. The other issue is creating a IOException with a TException as a parameter in writeObject and readObject. The attached patch creates a new option for the compiler, java5, which outputs java 5 compliant code. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
