Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Types used in lists must be declared beforehand #41

Closed
zhaakhi opened this issue Feb 22, 2019 · 1 comment
Closed

Types used in lists must be declared beforehand #41

zhaakhi opened this issue Feb 22, 2019 · 1 comment
Labels
bug Something isn't working

Comments

@zhaakhi
Copy link
Contributor

zhaakhi commented Feb 22, 2019

If you have a field with a type like list<T>, T must already have been declared, or the field will be serialized wrong.

How to reproduce:
In tests/container.thrift, move ListStruct to the top of the file, before ListItem,
Run the tests and test_container.py will fail:

    def test_list_struct():
        l_item = container.ListItem()
        l_item.list_string = ['foo', 'bar'] * 100
        l_item.list_list_string = [['foo', 'bar']] * 100
    
        l_struct = container.ListStruct()
        l_struct.list_items = [l_item] * 100
    
        b = serialize(l_struct)
        l_struct2 = deserialize(container.ListStruct(), b)
>       assert l_struct == l_struct2
E       AssertionError: assert ListStruct(list_items=[ListItem(list_string=['foo', 'bar', 'foo', 'bar', 'foo', 'bar', 'foo', 'bar', 'foo', 'bar', 'fo...r'], ['foo', 'bar'], ['foo', 'bar'], ['foo', 'bar'], ['foo', 'bar'], ['foo', 'bar'], ['foo', 'bar'], ['foo', 'bar']])])
== ListStruct(list_items=[None, None, None, None, None, None, None, None, None, None, None, None, None, None, None, None,...one, None, None, None, None, None, None, None, None, None, None, None, None, None, None, None, None, None, None, None])

test_container.py:55: AssertionError

It looks like fill_incomplete_ttype should deal with declaration order, but misses the list field. I would also expect serializing or deserializing incomplete types to fail instead of silently substituting 'None', since this makes bugs hard to debug.

@ethe ethe added the bug Something isn't working label Feb 23, 2019
@ethe
Copy link
Member

ethe commented Feb 23, 2019

Thanks for reporting, I am turning to fix it.

ethe added a commit to ethe/thriftpy2 that referenced this issue Feb 23, 2019
@ethe ethe mentioned this issue Feb 23, 2019
@ethe ethe closed this as completed in #42 Feb 23, 2019
ethe added a commit that referenced this issue Feb 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants