[jira] [Comment Edited] (THRIFT-2642) Recursive structs don't work in python
[
https://issues.apache.org/jira/browse/THRIFT-2642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16075348#comment-16075348
]
Eric Conner edited comment on THRIFT-2642 at 7/5/17 8:27 PM:
-
Hi [~jensg],
Thanks for the follow up. This pull request
https://github.com/apache/thrift/pull/1293 is my attempt at a fix (which I
linked above). Can someone review it?
I also outlined the approach above. Copied here:
I followed the fbthrift approach. At a high level, the change
Moves all thrift_spec definitions to the end of the ttypes file. Or, in the
case of a service, after the service definition.
Changes the initial declaration of the spec for a TStruct to [, None]
Uses a method call ``fix_spec`` to fill in the spec to [,
.thrift_spec]
I should note that this does not solve the case of co-recursive structures, a
problem that fbthrift also suffers from. Co-recursive structs can be created
but they cannot be serialized or deserialized (leads to infinite loop).
was (Author: econner724):
Hi [~jensg],
Thanks for the follow up. This pull request
https://github.com/apache/thrift/pull/1293 is my attempt at a fix (which I
linked above). Can someone review it?
I also outlined the approach above. Copied here:
I followed the fbthrift approach. At a high level, the change
Moves all thrift_spec definitions to the end of the ttypes file. Or, in the
case of a service, after the service definition.
Changes the initial declaration of the spec for a TStruct to [, None]
Uses a method call ``fix_spec`` to fill in the spec to [,
.thrift_spec]
I should note that this does not solve the case of co-recursive structures, a
problem that fbthrift also suffers from.
> 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 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
> 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)
[jira] [Comment Edited] (THRIFT-2642) Recursive structs don't work in python
[
https://issues.apache.org/jira/browse/THRIFT-2642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16056824#comment-16056824
]
Eric Conner edited comment on THRIFT-2642 at 6/21/17 5:13 PM:
--
Hi everyone,
I finally had some time to take a stab at fixing this.
I followed the fbthrift approach. At a high level, the change:
# Moves all thrift_spec definitions to the end of the ttypes file. Or, in the
case of a service, after the service definition.
# Changes the initial declaration of the spec for a TStruct to [, None]
# Uses a method call ``fix_spec`` to fill in the spec to [,
.thrift_spec]
*Edit*:
Originally I had tried to keep spec as a tuple, but as far as I can tell it has
to be a list as it needs to be a reference type for recursive structures to
work. I added tests to SerializationTest.py. Note that co-recursive structures
do not work. There is an infinite loop when they are serialized. fbthrift also
suffers from this problem.
Thanks [~lyschoening], [~myTalala], [~isanych], [~juliengreard], [~jensg]
was (Author: econner724):
Hi everyone,
I finally had some time to take a stab at fixing this.
I followed the fbthrift approach. 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 the spec for a TStruct to [, None]
# Uses a method call ``fix_spec`` to fill in the spec to [,
.thrift_spec]
*Edit*:
Originally I had tried to keep spec as a tuple, but as far as I can tell it has
to be a list as it needs to be a reference type for recursive structures to
work. I added tests to SerializationTest.py. Note that co-recursive structures
do not work. There is an infinite loop when they are serialized. fbthrift also
suffers from this problem.
Thanks [~lyschoening], [~myTalala], [~isanych], [~juliengreard], [~jensg]
> 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 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
> 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)
[jira] [Comment Edited] (THRIFT-2642) Recursive structs don't work in python
[
https://issues.apache.org/jira/browse/THRIFT-2642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16056824#comment-16056824
]
Eric Conner edited comment on THRIFT-2642 at 6/21/17 5:06 PM:
--
Hi everyone,
I finally had some time to take a stab at fixing this.
I followed the fbthrift approach. 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 the spec for a TStruct to [, None]
# Uses a method call ``fix_spec`` to fill in the spec to [,
.thrift_spec]
*Edit*:
Originally I had tried to keep spec as a tuple, but as far as I can tell it has
to be a list as it needs to be a reference type for recursive structures to
work. I added tests to SerializationTest.py. Note that co-recursive structures
do not work. There is an infinite loop when they are serialized. fbthrift also
suffers from this problem.
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 (, None)
# Uses a method call ``fix_spec`` to fill in the spec to (,
.thrift_spec)
The fbthrift approach is similar except their struct definition uses a list
[, .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]
> Recursive structs don't work in python
> --
>
> Key: THRIFT-2642
> URL:
[jira] [Comment Edited] (THRIFT-2642) Recursive structs don't work in python
[
https://issues.apache.org/jira/browse/THRIFT-2642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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 (, None)
# Uses a method call ``fix_spec`` to fill in the spec to (,
.thrift_spec)
The fbthrift approach is similar except their struct definition uses a list
[, .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 (, None)
# Uses a method call ``fix_spec`` to fill in the spec to (,
.thrift_spec)
The fbthrift approach is similar except their struct definition uses a list
[, .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
[jira] [Comment Edited] (THRIFT-2642) Recursive structs don't work in python
[
https://issues.apache.org/jira/browse/THRIFT-2642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16056824#comment-16056824
]
Eric Conner edited comment on THRIFT-2642 at 6/21/17 2:16 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 (, None)
# Uses a method call ``fix_spec`` to fill in the spec to (,
.thrift_spec)
The fbthrift approach is similar except their struct definition uses a list
[, .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]
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 (, None)
# Uses a method call ``fix_spec`` to fill in the spec to (,
.thrift_spec)
The fbthrift approach is similar except their struct definition uses a list
[, .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
[jira] [Comment Edited] (THRIFT-2642) Recursive structs don't work in python
[
https://issues.apache.org/jira/browse/THRIFT-2642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16056824#comment-16056824
]
Eric Conner edited comment on THRIFT-2642 at 6/21/17 2:03 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 (, None)
# Uses a method call ``fix_spec`` to fill in the spec to (,
.thrift_spec)
The fbthrift approach is similar except their struct definition uses a list
[, .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}
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 (, None)
# Uses a method call ``fix_spec`` to fill in the spec to (,
.thrift_spec)
The fbthrift approach is similar except their struct definition uses a list
[, .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
[jira] [Comment Edited] (THRIFT-2642) Recursive structs don't work in python
[
https://issues.apache.org/jira/browse/THRIFT-2642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16008223#comment-16008223
]
Jens Geyer edited comment on THRIFT-2642 at 5/12/17 2:40 PM:
-
[~allengeorge],
I am fine with it as long as
- the proposed changes are local to Python stuff (.i.e. there are no changes
necessary that affect other languages or question the infrastructure in
general)
- [~lyschoening], [~myTalala], [~econner724], [~isanych] and [~juliengreard]
agree with your pull request
- all builds are fine
I can surely do a general review, but my Python expertise is limited, so I
would not rely on my own opinion alone. Especially not with such a topic. But
if you 5 guys come to someething you are happy with as a group, we should have
enough backing to get it merged. At the end, this is open source.
was (Author: jensg):
[~allengeorge],
I am fine with it as long as
a) the proposed changes are local to Python stuff (.i.e. there are no changes
necessary that affect other languages or question the infrastructure in
general)
b) if [~lyschoening], [~myTalala], [~econner724] and [~juliengreard] agree with
your pull request
c) all builds are fine
I can surely do a general review, but my Python expertise is limited, so I
would not rely on my own opinion alone. Especially not with such a topic. But
if you 5 guys come to someething you are happy with as a group, we should have
enough backing to get it merged. At the end, this is open source.
> 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 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
> 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.3.15#6346)
[jira] [Comment Edited] (THRIFT-2642) Recursive structs don't work in python
[
https://issues.apache.org/jira/browse/THRIFT-2642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15387504#comment-15387504
]
Igor Rogozhin edited comment on THRIFT-2642 at 7/22/16 1:41 PM:
Hello from 2016.
We still have this problem.
I am using Julien's solution, but this kinda weird.
For full automation it required bash script, that's change this errors.
Maybe someone figure out how we can do it better?
EDIT:
Do we need "Recursive.thrift_spec"?
This kinda werid. I remove this on my project (and also fix following code for
checking is None).
was (Author: mytalala):
Hello from 2016.
We still have this problem.
I am using Julien's solution, but this kinda weird.
For full automation it required bash script, that's change this errors.
Maybe someone figure out how we can do it bett
> 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 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
> 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.3.4#6332)
[jira] [Comment Edited] (THRIFT-2642) Recursive structs don't work in python
[
https://issues.apache.org/jira/browse/THRIFT-2642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14970521#comment-14970521
]
Eric Conner edited comment on THRIFT-2642 at 10/23/15 6:21 AM:
---
The problem is in thrift_spec which is used only by fastbinary. Right now the
only option seems to be to manually modify the code as Julien has suggested.
As far as fixing this for fastbinary it gets tricky.
1) The thrift_spec either needs to move from the class level into a method or
thrift_spec should be added to the class after it is defined.
2) As written the thrift_spec tuple tries to be self-referential which is not
really possible in python unless you create lots of inner copies of the tuple.
It'd work in C with a pointer back to the object since fastbinary is really
just reading the tuple to see what fields to expect. It may be possible to
modify fastbinary to handle this case specifically, but the generated python
would also need to change as well and its not clear to me how to do that.
Maybe just using some sentinel value that fastbinary looks for which means "ref
to my parent".
was (Author: econner724):
The problem is in thrift_spec which is used only by fastbinary. Right now the
only option seems to be to manually modify the code as Julien has suggested:
A.thrift_spec = None
for idx in xrange(10):
A.thrift_spec = (
1, TType.STRUCT, 'field', (A, A.thrift_spec), None, )
)
As far as fixing this for fastbinary it gets tricky.
1) The thrift_spec either needs to move from the class level into a method or
thrift_spec should be added to the class after it is defined.
2) As written the thrift_spec tuple tries to be self-referential which is not
really possible in python unless you create lots of inner copies of the tuple.
It'd work in C with a pointer back to the object since fastbinary is really
just reading the tuple to see what fields to expect. It may be possible to
modify fastbinary to handle this case specifically, but the generated python
would also need to change as well and its not clear to me how to do that.
Maybe just using some sentinel value that fastbinary looks for which means "ref
to my parent".
> 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 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
> 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.3.4#6332)
[jira] [Comment Edited] (THRIFT-2642) Recursive structs don't work in python
[
https://issues.apache.org/jira/browse/THRIFT-2642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14970521#comment-14970521
]
Eric Conner edited comment on THRIFT-2642 at 10/23/15 6:22 AM:
---
The problem is in thrift_spec which is used only by fastbinary. Right now the
only option seems to be to manually modify the code as Julien has suggested.
As far as fixing this for fastbinary it gets tricky.
1) The thrift_spec either needs to move from the class level into a method or
thrift_spec should be added to the class after it is defined.
2) As written the thrift_spec tuple tries to be self-referential which is not
really possible in python unless you create lots of inner copies of the tuple.
It'd work in C with a pointer back to the object since fastbinary is really
just reading the tuple to see what fields to expect. It may be possible to
modify fastbinary to handle this case specifically, but the generated python
would also need to change as well and its not clear to me how to do that.
Maybe just using some sentinel value that fastbinary looks for which means "ref
to myself".
was (Author: econner724):
The problem is in thrift_spec which is used only by fastbinary. Right now the
only option seems to be to manually modify the code as Julien has suggested.
As far as fixing this for fastbinary it gets tricky.
1) The thrift_spec either needs to move from the class level into a method or
thrift_spec should be added to the class after it is defined.
2) As written the thrift_spec tuple tries to be self-referential which is not
really possible in python unless you create lots of inner copies of the tuple.
It'd work in C with a pointer back to the object since fastbinary is really
just reading the tuple to see what fields to expect. It may be possible to
modify fastbinary to handle this case specifically, but the generated python
would also need to change as well and its not clear to me how to do that.
Maybe just using some sentinel value that fastbinary looks for which means "ref
to my parent".
> 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 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
> 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.3.4#6332)
[jira] [Comment Edited] (THRIFT-2642) Recursive structs don't work in python
[
https://issues.apache.org/jira/browse/THRIFT-2642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14539620#comment-14539620
]
julien greard edited comment on THRIFT-2642 at 5/13/15 7:42 AM:
I did my own work around:
in the example above, I replaced the code:
class Recursive:
thrift_spec = (
None, # 0
(1, TType.LIST, 'Children', (TType.STRUCT,(Recursive,
Recursive.thrift_spec)), None, ), # 1
)
by (sorry for the bad indent, I don't know how to have a nice display)
{code}
class Recursive:
pass
Recursive.thrift_spec = None
depth = 0
max_depth = 50
while (depth max_depth):
depth += 1
Recursive. thrift_spec = (
None, # 0
(1, TType.LIST, 'Children', (TType.STRUCT,(Recursive,
Recursive.thrift_spec)), None, ), # 1
)
{code}
Which seems to do the job... It's ugly because I have to edit the generated
code afterward, but I didn't want to touch at the code generator in Thrift
code... what do you think ?
was (Author: juliengreard):
I did my own work around:
in the example above, I replaced the code:
class Recursive:
thrift_spec = (
None, # 0
(1, TType.LIST, 'Children', (TType.STRUCT,(Recursive,
Recursive.thrift_spec)), None, ), # 1
)
by (sorry for the bad indent, I don't know how to have a nice display)
class Recursive:
pass
Recursive.thrift_spec = None
depth = 0
max_depth = 50
while (depth max_depth):
depth += 1
Recursive. thrift_spec = (
None, # 0
(1, TType.LIST, 'Children', (TType.STRUCT,(Recursive,
Recursive.thrift_spec)), None, ), # 1
)
Which seems to do the job... It's ugly because I have to edit the generated
code afterward, but I didn't want to touch at the code generator in Thrift
code... what do you think ?
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: listRecursive 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.3.4#6332)
[jira] [Comment Edited] (THRIFT-2642) Recursive structs don't work in python
[
https://issues.apache.org/jira/browse/THRIFT-2642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14539620#comment-14539620
]
julien greard edited comment on THRIFT-2642 at 5/13/15 7:42 AM:
I did my own work around:
in the example above, I replaced the code:
{code}
class Recursive:
thrift_spec = (
None, # 0
(1, TType.LIST, 'Children', (TType.STRUCT,(Recursive,
Recursive.thrift_spec)), None, ), # 1
)
{code}
by
{code}
class Recursive:
pass
Recursive.thrift_spec = None
depth = 0
max_depth = 50
while (depth max_depth):
depth += 1
Recursive. thrift_spec = (
None, # 0
(1, TType.LIST, 'Children', (TType.STRUCT,(Recursive,
Recursive.thrift_spec)), None, ), # 1
)
{code}
Which seems to do the job... It's ugly because I have to edit the generated
code afterward, but I didn't want to touch at the code generator in Thrift
code... what do you think ?
was (Author: juliengreard):
I did my own work around:
in the example above, I replaced the code:
class Recursive:
thrift_spec = (
None, # 0
(1, TType.LIST, 'Children', (TType.STRUCT,(Recursive,
Recursive.thrift_spec)), None, ), # 1
)
by (sorry for the bad indent, I don't know how to have a nice display)
{code}
class Recursive:
pass
Recursive.thrift_spec = None
depth = 0
max_depth = 50
while (depth max_depth):
depth += 1
Recursive. thrift_spec = (
None, # 0
(1, TType.LIST, 'Children', (TType.STRUCT,(Recursive,
Recursive.thrift_spec)), None, ), # 1
)
{code}
Which seems to do the job... It's ugly because I have to edit the generated
code afterward, but I didn't want to touch at the code generator in Thrift
code... what do you think ?
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: listRecursive 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.3.4#6332)
[jira] [Comment Edited] (THRIFT-2642) Recursive structs don't work in python
[
https://issues.apache.org/jira/browse/THRIFT-2642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14540703#comment-14540703
]
Jens Geyer edited comment on THRIFT-2642 at 5/12/15 8:54 PM:
-
Hi,
there is a help icon which lists all the possible formattings.
Any chance that you could create a patch out of your findings and/or workaround
for the Python generator? We accept patch files and pull requests, see
[here|http://thrift.apache.org/docs/HowToContribute] for details.
was (Author: jensg):
Hi,
there is a help icon which lists all the possible formattings.
Any chance that you coulod create a patch out of your findings and/or
workaround, or does that make no sense?
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: listRecursive 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.3.4#6332)
[jira] [Comment Edited] (THRIFT-2642) Recursive structs don't work in python
[
https://issues.apache.org/jira/browse/THRIFT-2642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14539620#comment-14539620
]
julien greard edited comment on THRIFT-2642 at 5/12/15 10:14 AM:
-
I did my own work around:
in the example above, I replaced the code:
class Recursive:
thrift_spec = (
None, # 0
(1, TType.LIST, 'Children', (TType.STRUCT,(Recursive,
Recursive.thrift_spec)), None, ), # 1
)
by
class Recursive:
pass
Recursive.thrift_spec = None
depth = 0
max_depth = 50
while (depth max_depth):
depth += 1
Recursive. thrift_spec = (
None, # 0
(1, TType.LIST, 'Children', (TType.STRUCT,(Recursive,
Recursive.thrift_spec)), None, ), # 1
)
Which seems to do the job... It's ugly because I have to edit the generated
code afterward, but I didn't want to touch at the code generator in Thrift
code... what do you think ?
was (Author: juliengreard):
I did my own work around:
in the example above, I replaced the code:
class Recursive:
thrift_spec = (
None, # 0
(1, TType.LIST, 'Children', (TType.STRUCT,(Recursive,
Recursive.thrift_spec)), None, ), # 1
)
by
class Recursive:
pass
Recursive.thrift_spec = None
depth = 0
max_depth = 50
while (depth max_depth):
depth += 1
Recursive. thrift_spec = (
None, # 0
(1, TType.LIST, 'Children', (TType.STRUCT,(Recursive,
Recursive.thrift_spec)), None, ), # 1
)
Which seems to do the job... It's ugly because I have to edit the generated
code afterward, but I didn't want to touch at the code generator in Thrift
code... what do you think ?
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:
{quote}struct Recursive {
1: listRecursive Children
}
{quote}
Python code:
{quote}class Recursive:
thrift_spec = (
None, # 0
(1, TType.LIST, 'Children', (TType.STRUCT,(Recursive,
Recursive.thrift_spec)), None, ), # 1
)
{quote}
An error:
{quote}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
{quote}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Comment Edited] (THRIFT-2642) Recursive structs don't work in python
[
https://issues.apache.org/jira/browse/THRIFT-2642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14539620#comment-14539620
]
julien greard edited comment on THRIFT-2642 at 5/12/15 10:14 AM:
-
I did my own work around:
in the example above, I replaced the code:
class Recursive:
thrift_spec = (
None, # 0
(1, TType.LIST, 'Children', (TType.STRUCT,(Recursive,
Recursive.thrift_spec)), None, ), # 1
)
by (sorry for the bad indent, I don't know how to have a nice display)
class Recursive:
pass
Recursive.thrift_spec = None
depth = 0
max_depth = 50
while (depth max_depth):
depth += 1
Recursive. thrift_spec = (
None, # 0
(1, TType.LIST, 'Children', (TType.STRUCT,(Recursive,
Recursive.thrift_spec)), None, ), # 1
)
Which seems to do the job... It's ugly because I have to edit the generated
code afterward, but I didn't want to touch at the code generator in Thrift
code... what do you think ?
was (Author: juliengreard):
I did my own work around:
in the example above, I replaced the code:
class Recursive:
thrift_spec = (
None, # 0
(1, TType.LIST, 'Children', (TType.STRUCT,(Recursive,
Recursive.thrift_spec)), None, ), # 1
)
by
class Recursive:
pass
Recursive.thrift_spec = None
depth = 0
max_depth = 50
while (depth max_depth):
depth += 1
Recursive. thrift_spec = (
None, # 0
(1, TType.LIST, 'Children', (TType.STRUCT,(Recursive,
Recursive.thrift_spec)), None, ), # 1
)
Which seems to do the job... It's ugly because I have to edit the generated
code afterward, but I didn't want to touch at the code generator in Thrift
code... what do you think ?
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:
{quote}struct Recursive {
1: listRecursive Children
}
{quote}
Python code:
{quote}class Recursive:
thrift_spec = (
None, # 0
(1, TType.LIST, 'Children', (TType.STRUCT,(Recursive,
Recursive.thrift_spec)), None, ), # 1
)
{quote}
An error:
{quote}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
{quote}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
