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

Will Pierce commented on THRIFT-1088:
-------------------------------------

This patch works fine in my own application code.

I just learned how to run the test suite in the trunk/, which is a win, but a 
bigger win is that all tests passed from running 'make check' in the 
trunk/test/ directory. (see output below)

This was my first ever attempt to run the thrift tests, so I'd appreciate if 
someone more experienced than me could confirm that the changes pass all tests.

My test output (grepped):
{noformat}
% make check 2>&1 | egrep  '^All|Ran|OK|PASS|FAIL|==='
Ran 8 tests in 0.002s
OK
PASS: SerializationTest.py
Ran 4 tests in 0.002s
OK
PASS: TestEof.py
PASS: TestSyntax.py
Ran 20 tests in 0.018s
OK
Ran 20 tests in 0.022s
OK
Ran 20 tests in 0.021s
OK
Ran 20 tests in 0.042s
OK
Ran 20 tests in 0.027s
OK
Ran 20 tests in 0.043s
OK
PASS: RunClientServer.py
==================
All 4 tests passed
==================
{noformat}



> Thrift library python classes should be new-style classes
> ---------------------------------------------------------
>
>                 Key: THRIFT-1088
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1088
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Python - Library
>    Affects Versions: 0.6
>            Reporter: Will Pierce
>            Priority: Minor
>         Attachments: THRIFT-1088_pthon_new_style_classes.patch
>
>
> The included python thrift library classes (TType, TProtocolBase, etc...) are 
> all defined using old-style python class definitions.  This makes it somewhat 
> more difficult to dynamically deal with instances of those classes at 
> runtime, since the type() builtin doesn't return the class name.  I don't 
> think there's really any good reason for avoiding using new style classes.  
> The typical fear/uncertainty/doubt with new-style classes (since 2001 when 
> they were made available in python 2.2) is that they are thought to be slower 
> than old-style classes.  I personally haven't ever found the performance 
> difference between old and new style classes to 
> Unless there's a compelling reason that Thrift developers want to use 
> old-style classes, I think it's a more programmer-friendly choice to switch 
> these to new-style classes.  Not to mention the benefits that new style 
> classes provide, see: 
> http://realmike.org/blog/2010/07/18/introduction-to-new-style-classes-in-python/
>  
> The following thrift classes don't inherit from the 'object' class, so are 
> old-style classes:
> {noformat}
> % find . -name \*py | xargs grep class | grep -v [.]svn | grep '[a-zA-Z0-9]:$'
> ./src/protocol/TCompactProtocol.py:class CompactType:
> ./src/protocol/TCompactProtocol.py:class TCompactProtocolFactory:
> ./src/protocol/TProtocol.py:class TProtocolBase:
> ./src/protocol/TProtocol.py:class TProtocolFactory:
> ./src/protocol/TBinaryProtocol.py:class TBinaryProtocolFactory:
> ./src/protocol/TBinaryProtocol.py:class TBinaryProtocolAcceleratedFactory:
> ./src/Thrift.py:class TType:
> ./src/Thrift.py:class TMessageType:
> ./src/Thrift.py:class TProcessor:
> ./src/server/TServer.py:class TServer:
> ./src/server/TNonblockingServer.py:class Connection:
> ./src/server/TNonblockingServer.py:class TNonblockingServer:
> ./src/transport/TTransport.py:class TTransportBase:
> ./src/transport/TTransport.py:class CReadableTransport:
> ./src/transport/TTransport.py:class TServerTransportBase:
> ./src/transport/TTransport.py:class TTransportFactoryBase:
> ./src/transport/TTransport.py:class TBufferedTransportFactory:
> ./src/transport/TTransport.py:class TFramedTransportFactory:
> {noformat}
> I'll make changes to my local copy of trunk, run the tests and then attach a 
> patch to this ticket for evaluation.
> Unless anyone has objections or concerns about switching to new style classes?

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to