[ https://issues.apache.org/jira/browse/THRIFT-2642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16056824#comment-16056824 ]
Eric Conner edited comment on THRIFT-2642 at 6/21/17 2:17 AM: -------------------------------------------------------------- Hi everyone, I finally had some time to take a stab at fixing this. I more or less followed the fbthrift approach with a few modifications. At a high level, the change: # Moves all thrift_spec definitions to the end of the ttypes or service file. # Changes the initial declaration of thrift_spec to (<Struct>, None) # Uses a method call ``fix_spec`` to fill in the spec to (<Struct>, <Struct>.thrift_spec) The fbthrift approach is similar except their struct definition uses a list [<Struct>, <Struct>.thrift_spec]. I looked at this route for awhile, but Apache Thrift's fastbinary implementation depends pretty heavily on ``thrift_spec`` being immutable. There are ref counting assumptions I don't fully understand that led to lots of complications when I tried to change things to use lists (see https://github.com/apache/thrift/blob/19baeefd8c38d62085891d7956349601f79448b3/lib/py/src/ext/protocol.tcc#L341). Instead, I'm using a modified version of fbthrift's fix_spec script (https://github.com/facebook/fbthrift/blob/3ef868c969464e935b39e336807af1486b2739c3/thrift/lib/py/util/Recursive.py). The main change I made was to convert each spec to a list, modify the list recursively, and then convert the list back to a tuple. I'm unsure of the performance hit this adds, but I figured it wouldn't be too bad since it only happens at import time. Finally, I had *lots* of trouble trying to figure out how to add tests for this to the python test harness. I wanted to add Recursive.thrift to the generated test code for python and then add a basic test which uses it. I could not figure out how to add Recursive.thrift to the gen-py makefile. It seems like the files that need to change are Makefile.am and generate.cmake within thrift/test/py. When I do that the only way I could get the code generated was by re-running bootstrap.sh, configure, and make which has like a 10 minute turnaround time on my machine. Surely there is a faster way to do that? Second, I was really unclear on what all the gen-py-* directories were for. Can anyone clarify that for me? Here is my external test script that I'd like to add: {code:python} import thrift from Recursive.ttypes import * def test_rec_tree(): print "Tree Test ..", children = [] for idx in xrange(1, 5): node = RecTree(idx) children.append(node) parent = RecTree(item=0, children=children) assert len(parent.children) == 4 for child in parent.children: assert isinstance(child, RecTree) print "OK" def _get_linked_list(): head = cur = RecList(item=0) for idx in xrange(1, 5): node = RecList(item=idx) cur.nextitem = node cur = node return head def _collapse_linked_list(head): out_list = [] cur = head while cur is not None: out_list.append(cur.item) cur = cur.nextitem return out_list def test_rec_list(): print "Linked List Test ..", head = _get_linked_list() golden_list = [0, 1, 2, 3, 4] test_list = _collapse_linked_list(head) assert golden_list == test_list print "OK" def test_co_rec(): print "Co Rec Test ..", item1 = CoRec() item2 = CoRec2() item1.other = item2 item2.other = item1 print "OK" def test_vector(): print "Vector Test ..", mylist = [_get_linked_list(), _get_linked_list()] myvec = VectorTest(lister=mylist) golden_list = [0, 1, 2, 3, 4] for cur_list in myvec.lister: out_list = _collapse_linked_list(cur_list) assert golden_list == out_list print "OK" if __name__ == "__main__": test_rec_tree() test_rec_list() test_co_rec() test_vector() {code} Thanks [~lyschoening], [~myTalala], [~isanych], [~juliengreard], [~jensg] was (Author: econner724): Hi everyone, I finally had some time to take a stab at fixing this. I more or less followed the fbthrift approach with a few modifications. At a high level, the change: # Moves all thrift_spec definitions to the end of the ttypes or service file. # Changes the initial declaration of thrift_spec to (<Struct>, None) # Uses a method call ``fix_spec`` to fill in the spec to (<Struct>, <Struct>.thrift_spec) The fbthrift approach is similar except their struct definition uses a list [<Struct>, <Struct>.thrift_spec]. I looked at this route for awhile, but Apache Thrift's fastbinary implementation depends pretty heavily on ``thrift_spec`` being immutable. There are ref counting assumptions I don't fully understand that led to lots of complications when I tried to change things to use lists (see https://github.com/apache/thrift/blob/19baeefd8c38d62085891d7956349601f79448b3/lib/py/src/ext/protocol.tcc#L341). Instead, I'm using a modified version of fbthrift's fix_spec script (https://github.com/facebook/fbthrift/blob/3ef868c969464e935b39e336807af1486b2739c3/thrift/lib/py/util/Recursive.py). The main change I made was to convert each spec to a list, modify the list recursively, and then convert the list back to a tuple. I'm unsure of the performance hit this adds, but I figured it wouldn't be too bad since it only happens at import time. Finally, I had *lots* of trouble trying to figure out how to add tests for this to the python test harness. I wanted to add Recursive.thrift to the generated test code for python and then add a basic test which uses it. I could not figure out how to add Recursive.thrift to the gen-py makefile. It seems like the files that need to change are Makefile.am and generate.cmake within thrift/test/py. When I do that the only way I could get the code generated was by re-running bootstrap.sh, configure, and make which has like a 10 minute turnaround time on my machine. Surely there is a faster way to do that? Second, I was really unclear on what all the gen-py-* directories were for. Can anyone clarify that for me? Here is my external test script that I'd like to add: {code:python} import thrift from Recursive.ttypes import * def test_rec_tree(): print "Tree Test ..", children = [] for idx in xrange(1, 5): node = RecTree(idx) children.append(node) parent = RecTree(item=0, children=children) assert len(parent.children) == 4 for child in parent.children: assert isinstance(child, RecTree) print "OK" def _get_linked_list(): head = cur = RecList(item=0) for idx in xrange(1, 5): node = RecList(item=idx) cur.nextitem = node cur = node return head def _collapse_linked_list(head): out_list = [] cur = head while cur is not None: out_list.append(cur.item) cur = cur.nextitem return out_list def test_rec_list(): print "Linked List Test ..", head = _get_linked_list() golden_list = [0, 1, 2, 3, 4] test_list = _collapse_linked_list(head) assert golden_list == test_list print "OK" def test_co_rec(): print "Co Rec Test ..", item1 = CoRec() item2 = CoRec2() item1.other = item2 item2.other = item1 print "OK" def test_vector(): print "Vector Test ..", mylist = [_get_linked_list(), _get_linked_list()] myvec = VectorTest(lister=mylist) golden_list = [0, 1, 2, 3, 4] for cur_list in myvec.lister: out_list = _collapse_linked_list(cur_list) assert golden_list == out_list print "OK" if __name__ == "__main__": test_rec_tree() test_rec_list() test_co_rec() test_vector() {code} Thanks [~lyschoening], [~myTalala], [~isanych], [~juliengreard] > Recursive structs don't work in python > -------------------------------------- > > Key: THRIFT-2642 > URL: https://issues.apache.org/jira/browse/THRIFT-2642 > Project: Thrift > Issue Type: Bug > Components: Python - Compiler, Python - Library > Affects Versions: 0.9.2 > Reporter: Igor Kostenko > > Recursive structs in 0.9.2 work fine in c++ & c#, but not in python, because > generated code trying to use objects which not constructed yet. > Struct: > {code} > struct Recursive { > 1: list<Recursive> Children > } > {code} > Python code: > {code} > class Recursive: > thrift_spec = ( > None, # 0 > (1, TType.LIST, 'Children', (TType.STRUCT,(Recursive, > Recursive.thrift_spec)), None, ), # 1 > ) > {code} > Error message: > {code} > Traceback (most recent call last): > File "ttypes.py", line 20, in <module> > class Recursive: > File "ttypes.py", line 28, in Recursive > (1, TType.LIST, 'Children', (TType.STRUCT,(Recursive, > Recursive.thrift_spec)), None, ), # 1 > NameError: name 'Recursive' is not defined > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)