Thrift
  1. Thrift
  2. THRIFT-162

Thrift structures are unhashable, preventing them from being used as set elements

    Details

    • Patch Info:
      Patch Available

      Description

      Let Foo be a Thrift structure:

      struct Foo

      { 1: i32 bar }

      If you want to use it properly as a set element or a as a dictionary key, the autoegenerated Python code will complain about not being hashable:

      >>> f1 = Foo()
      >>> f1.bar = 1
      >>> f2 = Foo()
      >>> f2.bar = 1
      >>> f1 == f2
      True
      >>> set([f1]) & set([f2])
      set([])
      >>> d = {}
      >>> d[f1] = 2
      Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      TypeError: unhashable instance

      Since Thrift structures already implement _eq_ and _ne, they should implement __hash_ as well. The attached patch tries to mimic the behaviour of the Java compiler, including a HashCodeBuilder class written in Python.

      1. thrift-162_v2_annotiations.patch
        5 kB
        Esteve Fernandez
      2. thrift-162_annotiations.patch
        2 kB
        Esteve Fernandez
      3. thrift_py_hash.patch
        4 kB
        Esteve Fernandez
      4. reading-containers.diff
        3 kB
        David Reiss
      5. immut-v4.diff
        7 kB
        David Reiss
      6. immut-v3.diff
        9 kB
        David Reiss
      7. immut-v2.diff
        7 kB
        David Reiss
      8. immut-no-slots-v1.diff
        2 kB
        David Reiss

        Issue Links

          Activity

          Hide
          Ashutosh Chauhan added a comment -

          We got hit by this too. We tried to workaround this issue HIVE-4322 Than failed HIVE-4432 and HIVE-4433 Subsequently we had to roll-back this at 11th hour HIVE-4492 to unblock a release. Suffice to say it hit us badly. Would love to see this fixed in core thrift.

          Show
          Ashutosh Chauhan added a comment - We got hit by this too. We tried to workaround this issue HIVE-4322 Than failed HIVE-4432 and HIVE-4433 Subsequently we had to roll-back this at 11th hour HIVE-4492 to unblock a release. Suffice to say it hit us badly. Would love to see this fixed in core thrift.
          Hide
          Bryan Duxbury added a comment -

          I'm untagging this for 0.4 because I don't expect it to get done today, but I'd really love it if someone who understands the final state of this issue were to figure it out and either commit it or let me know so I can commit it.

          Show
          Bryan Duxbury added a comment - I'm untagging this for 0.4 because I don't expect it to get done today, but I'd really love it if someone who understands the final state of this issue were to figure it out and either commit it or let me know so I can commit it.
          Hide
          David Reiss added a comment -

          This additional patch provides support for reading immutable containers with normal (not fastbinary) protocols. I think we're going to have to extend thrift_spec again to make fastbinary work.

          Show
          David Reiss added a comment - This additional patch provides support for reading immutable containers with normal (not fastbinary) protocols. I think we're going to have to extend thrift_spec again to make fastbinary work.
          Hide
          David Reiss added a comment -

          Sorry, diffed against the wrong branch.

          Show
          David Reiss added a comment - Sorry, diffed against the wrong branch.
          Hide
          Nicolas Trangez added a comment -

          immut-v3.diff seems to contain some unrelated hunks (reverts even, I think) in fastbinary.c, the Ruby lib and some testcases.

          Show
          Nicolas Trangez added a comment - immut-v3.diff seems to contain some unrelated hunks (reverts even, I think) in fastbinary.c, the Ruby lib and some testcases.
          Hide
          David Reiss added a comment -

          @Esteve: I don't like named tuples for this case because they require different code to use compared to normal dicts and they are very awkward if keys are non-identifiers (or non-strings!).

          @Nicholas: Good catch. Thrift structures are not sortable in Python, but I can sort the items by hash. New patch coming soon.

          Show
          David Reiss added a comment - @Esteve: I don't like named tuples for this case because they require different code to use compared to normal dicts and they are very awkward if keys are non-identifiers (or non-strings!). @Nicholas: Good catch. Thrift structures are not sortable in Python, but I can sort the items by hash. New patch coming soon.
          Hide
          Nicolas Trangez added a comment -

          As I see it in http://gitweb.thrift-rpc.org/?p=thrift.git;a=commitdiff;h=bccfbe9a6ea9a1fee27f3b201befcbf034090c97, 2 TFreezableDicts containing the same items can have another hash value depending on insertion order since the hash is calculated based on the return value of dict.items(), which is not ordered.

          Shouldn't we sort this somehow before calculating the result hash?

          Show
          Nicolas Trangez added a comment - As I see it in http://gitweb.thrift-rpc.org/?p=thrift.git;a=commitdiff;h=bccfbe9a6ea9a1fee27f3b201befcbf034090c97 , 2 TFreezableDicts containing the same items can have another hash value depending on insertion order since the hash is calculated based on the return value of dict.items(), which is not ordered. Shouldn't we sort this somehow before calculating the result hash?
          Hide
          Esteve Fernandez added a comment - - edited

          I still prefer adding slots, but I guess that could be discussed at thrift-dev. However, I'm not sure I like your implementation of const maps, I'd rather backport named tuples from Python 2.6, here's the recipe:

          http://code.activestate.com/recipes/500261/

          It should be fairly easy to check whether we should use collections.namedtuple (2.6) or our backported version. Something like:

          import sys
          if sys.hexversion >= 0x20600f0:
              from collections import namedtuple
          else:
              from thrift.compat import namedtuple
          

          BTW, I'm sorry, but I won't be able to devote too much time to this issue (and to Thrift in general) during the next days Feel free to take over it David.

          Show
          Esteve Fernandez added a comment - - edited I still prefer adding slots, but I guess that could be discussed at thrift-dev. However, I'm not sure I like your implementation of const maps, I'd rather backport named tuples from Python 2.6, here's the recipe: http://code.activestate.com/recipes/500261/ It should be fairly easy to check whether we should use collections.namedtuple (2.6) or our backported version. Something like: import sys if sys.hexversion >= 0x20600f0: from collections import namedtuple else : from thrift.compat import namedtuple BTW, I'm sorry, but I won't be able to devote too much time to this issue (and to Thrift in general) during the next days Feel free to take over it David.
          Hide
          David Reiss added a comment -

          This version makes the syntax test pass. Yay! Of course, reading still doesn't work.
          A semi-coherent breakdown of the changes is here: http://gitweb.thrift-rpc.org/?p=thrift.git;a=log;h=refs/heads/pri/dreiss/py-immut;hb=HEAD

          Show
          David Reiss added a comment - This version makes the syntax test pass. Yay! Of course, reading still doesn't work. A semi-coherent breakdown of the changes is here: http://gitweb.thrift-rpc.org/?p=thrift.git;a=log;h=refs/heads/pri/dreiss/py-immut;hb=HEAD
          Hide
          David Reiss added a comment -

          Okay. Here is my version without the slots stuff. Slots don't prevent you from doing

              object.__setattr__(my_immut, 'field', value)
          

          so a really crafty hacker can still break his own program. Therefore, there is not much need to block _dict_.

          Obviously the patch is much simpler. We can still consider using slots, but I think it should be a separate discussion, and it is lower priority.

          We still have to implement immutable lists, sets, and (gasp) maps, both in the readers and render_const_value before this will actually work, though.

          Show
          David Reiss added a comment - Okay. Here is my version without the slots stuff. Slots don't prevent you from doing object.__setattr__(my_immut, 'field', value) so a really crafty hacker can still break his own program. Therefore, there is not much need to block _ dict _. Obviously the patch is much simpler. We can still consider using slots, but I think it should be a separate discussion, and it is lower priority. We still have to implement immutable lists, sets, and (gasp) maps, both in the readers and render_const_value before this will actually work, though.
          Hide
          David Reiss added a comment -

          I'm working on this now. Can you explain this comment?

              for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
                // This fills in default values, as opposed to nulls
                out << "self." << (*m_iter)->get_name() << ", ";
              }
          
          
          Show
          David Reiss added a comment - I'm working on this now. Can you explain this comment? for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) { // This fills in default values, as opposed to nulls out << "self." << (*m_iter)->get_name() << ", " ; }
          Hide
          Esteve Fernandez added a comment -

          I had to add a _slots_ variable to prevent Python from creating a _dict_ instance, otherwise one could still modify an instance using obj._dict_["foo"] = 42

          As for the _init_ implementation, although it looks a bit awkward, it's necessary as it's impossible to call self.X = Y once _setattr_ is overloaded to raise an exception, so to set an attribute, I had to call _setattr_ using the parent class.

          Show
          Esteve Fernandez added a comment - I had to add a _ slots _ variable to prevent Python from creating a _ dict _ instance, otherwise one could still modify an instance using obj._ dict _ ["foo"] = 42 As for the _ init _ implementation, although it looks a bit awkward, it's necessary as it's impossible to call self.X = Y once _ setattr _ is overloaded to raise an exception, so to set an attribute, I had to call _ setattr _ using the parent class.
          Hide
          David Reiss added a comment -

          FWIW, I think having an annotation for immutability is the best way to achieve sets of structures and maps with structures as keys.

          Could you explain the addition of _slots? Is this related to the changes in repr and eq? Can you also explain the implementation of __init? If the __slots_ stuff is not strictly necessary, I'd prefer to see it in a separate diff so the initial code review will be easier and we can give it more attention.

          Show
          David Reiss added a comment - FWIW, I think having an annotation for immutability is the best way to achieve sets of structures and maps with structures as keys. Could you explain the addition of _ slots ? Is this related to the changes in repr and eq? Can you also explain the implementation of __init ? If the __slots _ stuff is not strictly necessary, I'd prefer to see it in a separate diff so the initial code review will be easier and we can give it more attention.
          Hide
          Esteve Fernandez added a comment -

          But then, this contradicts the purpose of type annotations. Taken from David's commit message for THRIFT-121, the usefulness of arbitrary type annotations is:

          "Adds syntax for attaching arbitrary key/value pairs to types.
          These annotations can be accessed by individual generators to alter
          the code they produce."

          For example, I have a patch that I'll try to submit tomorrow which adds support for using tuples instead of lists for the list<> container type. And, if I understood it correctly, the encouraged way to add such feature is to use annotations, instead of hacking a new special type in the IDL parser.

          Show
          Esteve Fernandez added a comment - But then, this contradicts the purpose of type annotations. Taken from David's commit message for THRIFT-121 , the usefulness of arbitrary type annotations is: "Adds syntax for attaching arbitrary key/value pairs to types. These annotations can be accessed by individual generators to alter the code they produce." For example, I have a patch that I'll try to submit tomorrow which adds support for using tuples instead of lists for the list<> container type. And, if I understood it correctly, the encouraged way to add such feature is to use annotations, instead of hacking a new special type in the IDL parser.
          Hide
          Ben Maurer added a comment -

          I think we should avoid having two different code generation paths – it's much more complex to test and will be prone to bugs.

          Show
          Ben Maurer added a comment - I think we should avoid having two different code generation paths – it's much more complex to test and will be prone to bugs.
          Hide
          Esteve Fernandez added a comment -

          Any opinion on whether the python.immutable annotation is a good thing or not? I think it could be standardized (java.immutable, ruby.immutable, etc.) and have immutable types across all languages, but would like to hear the opinion of others.

          Show
          Esteve Fernandez added a comment - Any opinion on whether the python.immutable annotation is a good thing or not? I think it could be standardized (java.immutable, ruby.immutable, etc.) and have immutable types across all languages, but would like to hear the opinion of others.
          Hide
          Esteve Fernandez added a comment -

          Fixes _repr_ and _eq_

          Show
          Esteve Fernandez added a comment - Fixes _ repr _ and _ eq _
          Hide
          Esteve Fernandez added a comment -

          Well, I've implemented immutable structures using annotations (python.immutable). I hope I haven't introduced any bugs, but I'd like someone else (David, Ben, etc.) to review my patch.

          Wow, the annotations stuff is really impressive and easy to use

          Show
          Esteve Fernandez added a comment - Well, I've implemented immutable structures using annotations (python.immutable). I hope I haven't introduced any bugs, but I'd like someone else (David, Ben, etc.) to review my patch. Wow, the annotations stuff is really impressive and easy to use
          Hide
          Esteve Fernandez added a comment -

          This patch uses the annotations mechanism to implement immutable structures.

          Show
          Esteve Fernandez added a comment - This patch uses the annotations mechanism to implement immutable structures.
          Hide
          Chad Walters added a comment -

          I have already proposed that we disallow non-basic types as map keys because there are several languages for which this is actually problematic (in fact, I thought that we had reached general agreement on that but perhaps I am mistaken).

          The C++ case you present w.r.t. comparators can be worked around, I believe, but these others cannot.

          I realize I haven't been able to participate as much as I would have liked over the past 6 months but I really strongly believe that we should be striving for interoperability as one of the foremost goals. Thrift has taken this approach in the past – for example, the decision to support only signed integer types. At the least, I'd suggest we try to have a broad discussion about this, since it seems like a key decision.

          Show
          Chad Walters added a comment - I have already proposed that we disallow non-basic types as map keys because there are several languages for which this is actually problematic (in fact, I thought that we had reached general agreement on that but perhaps I am mistaken). The C++ case you present w.r.t. comparators can be worked around, I believe, but these others cannot. I realize I haven't been able to participate as much as I would have liked over the past 6 months but I really strongly believe that we should be striving for interoperability as one of the foremost goals. Thrift has taken this approach in the past – for example, the decision to support only signed integer types. At the least, I'd suggest we try to have a broad discussion about this, since it seems like a key decision.
          Hide
          David Reiss added a comment -

          There is another case too. C++ can't use structures as map keys or set elements unless a comparator is defined. Rather than presume an ordering by defining one automatically, we leave it to application developers to define one if they need it. My point is that some data types simply can't be expressed naturally in a given language. Service writers will always have to consider how their data will appear in other languages. Rather than trying to shoehorn the cases we can, I think that it is best to accept that data types will be language-dependent and respect the expectations of programmers in each language.

          Show
          David Reiss added a comment - There is another case too. C++ can't use structures as map keys or set elements unless a comparator is defined. Rather than presume an ordering by defining one automatically, we leave it to application developers to define one if they need it. My point is that some data types simply can't be expressed naturally in a given language. Service writers will always have to consider how their data will appear in other languages. Rather than trying to shoehorn the cases we can, I think that it is best to accept that data types will be language-dependent and respect the expectations of programmers in each language.
          Hide
          David Reiss added a comment -

          How about using a list of ints or an int->int map as a map key? That won't work in Python even if we do define _hash_. What should we do about that?

          Show
          David Reiss added a comment - How about using a list of ints or an int->int map as a map key? That won't work in Python even if we do define _ hash _. What should we do about that?
          Hide
          Chad Walters added a comment -

          I respectfully disagree with David here. The goal of interoperability across languages supercedes the goal of being idiomatic in each language, at least in my eyes.

          Either we choose to violate Python's conventions here (effectively choosing Java's "trust-the-programmer" model) or we limit sets to operating only on basic types. Any kind of middle ground means that I have to anticipate whether or not any of my clients will be in Python when I decide whether to include a set of Thrift objects in my interface.

          Show
          Chad Walters added a comment - I respectfully disagree with David here. The goal of interoperability across languages supercedes the goal of being idiomatic in each language, at least in my eyes. Either we choose to violate Python's conventions here (effectively choosing Java's "trust-the-programmer" model) or we limit sets to operating only on basic types. Any kind of middle ground means that I have to anticipate whether or not any of my clients will be in Python when I decide whether to include a set of Thrift objects in my interface.
          Hide
          David Reiss added a comment -

          I disagree. I think that it is more important for Thrift to conform to the standard idioms of the individual languages than to be identical across languages. I don't know what the convention is in Ruby, but Java and Python are clear. Java defines hashCode/_hash_ for mutable objects and Python doesn't. I think we should stick to these conventions.

          Show
          David Reiss added a comment - I disagree. I think that it is more important for Thrift to conform to the standard idioms of the individual languages than to be identical across languages. I don't know what the convention is in Ruby, but Java and Python are clear. Java defines hashCode/_ hash _ for mutable objects and Python doesn't. I think we should stick to these conventions.
          Hide
          Esteve Fernandez added a comment -

          I'm reopening this issue, THRIFT-231 exposes the exact same problem and the solution was to make the hash method to always return 0

          I don't think it's the best solution, but if we're following this path, then we should make this behavior homogeneous (even if it's not completely right). I'd rather to use annotations, but it may take a while until I can upload a patch that implements immutability of Thrift structures through annotations.

          Show
          Esteve Fernandez added a comment - I'm reopening this issue, THRIFT-231 exposes the exact same problem and the solution was to make the hash method to always return 0 I don't think it's the best solution, but if we're following this path, then we should make this behavior homogeneous (even if it's not completely right). I'd rather to use annotations, but it may take a while until I can upload a patch that implements immutability of Thrift structures through annotations.
          Hide
          David Reiss added a comment -

          Sorry for the miscommunication. I didn't mean storing a dict in your structure. I meant replacing your set of objects with a dict from keys to objects.

          As for the immutable keyword, I do think that it would make sense as a type modifier (this infrastructure hasn't been implemented yet).

          Show
          David Reiss added a comment - Sorry for the miscommunication. I didn't mean storing a dict in your structure. I meant replacing your set of objects with a dict from keys to objects. As for the immutable keyword, I do think that it would make sense as a type modifier (this infrastructure hasn't been implemented yet).
          Hide
          Esteve Fernandez added a comment -

          From the Python docs: """If a class defines mutable objects and implements a cmp() or __eq() method, it should not implement __hash(), since the dictionary implementation requires that a key's hash value is immutable (if the object's hash value changes, it will be in the wrong hash bucket)."""

          Yes, I know, that's why I said that only immutable objects can have both _eq() and __hash_()

          Thrift structures are inherently mutable, so they should not define hash(). Please use a map with simple types as keys if you are using Python.

          No, using a map won't solve this problem. As I already said, dictionaries (maps) are mutable and thus, can't be used as set elements. I'll either use a tuple (i64, i64) or a tree-based set, such as ZODB's TreeSet (http://www.zope.org/Wikis/ZODB/guide/node6.html#SECTION000630000000000000000), it won't be O(1), though.

          Anyway, would it be useful to add an immutable keyword to Thrift structures in the IDL? This would make structures immutable (overriding _setattr_ in Python, removing setters in Java, declaring attributes with attr_reader, but not attr_writer in Ruby, etc.) I'm sure there are more implications, for example, one would have to make struct members immutable as well, but would like to know your opinions.

          Show
          Esteve Fernandez added a comment - From the Python docs: """If a class defines mutable objects and implements a cmp() or __eq() method, it should not implement __hash (), since the dictionary implementation requires that a key's hash value is immutable (if the object's hash value changes, it will be in the wrong hash bucket).""" Yes, I know, that's why I said that only immutable objects can have both _ eq () and __hash _() Thrift structures are inherently mutable, so they should not define hash (). Please use a map with simple types as keys if you are using Python. No, using a map won't solve this problem. As I already said, dictionaries (maps) are mutable and thus, can't be used as set elements. I'll either use a tuple (i64, i64) or a tree-based set, such as ZODB's TreeSet ( http://www.zope.org/Wikis/ZODB/guide/node6.html#SECTION000630000000000000000 ), it won't be O(1), though. Anyway, would it be useful to add an immutable keyword to Thrift structures in the IDL? This would make structures immutable (overriding _ setattr _ in Python, removing setters in Java, declaring attributes with attr_reader, but not attr_writer in Ruby, etc.) I'm sure there are more implications, for example, one would have to make struct members immutable as well, but would like to know your opinions.
          Hide
          David Reiss added a comment -

          I agree that it is an interesting problem, but I think disallowing non-base types in sets and map keys is overkill. C++ enforces the immutability of set members and map keys (as best as it can, given its explicit pointers and direct access to memory) using const. As far as I can tell, Java's convention is to trust the application developer not to mutate keys rather than to make mutable objects unhashable. In Erlang, (almost) all values are immutable/copy-on-write, so there is no problem using mutable objects as keys. The best thing I can think of would be to put some verification code in each generator to make sure you aren't trying to do something that isn't appropriate for the language you are generating.

          Show
          David Reiss added a comment - I agree that it is an interesting problem, but I think disallowing non-base types in sets and map keys is overkill. C++ enforces the immutability of set members and map keys (as best as it can, given its explicit pointers and direct access to memory) using const. As far as I can tell, Java's convention is to trust the application developer not to mutate keys rather than to make mutable objects unhashable. In Erlang, (almost) all values are immutable/copy-on-write, so there is no problem using mutable objects as keys. The best thing I can think of would be to put some verification code in each generator to make sure you aren't trying to do something that isn't appropriate for the language you are generating.
          Hide
          Bryan Duxbury added a comment -

          This is an interesting point, and one that should perhaps be generalized to Thrift as a whole. This same problem exists in Java and probably Ruby as well. Maybe we should disallow non-primitives as hash keys as part of the IDL.

          Show
          Bryan Duxbury added a comment - This is an interesting point, and one that should perhaps be generalized to Thrift as a whole. This same problem exists in Java and probably Ruby as well. Maybe we should disallow non-primitives as hash keys as part of the IDL.
          Hide
          David Reiss added a comment -

          From the Python docs: """If a class defines mutable objects and implements a _cmp() or __eq() method, it should not implement __hash_(), since the dictionary implementation requires that a key's hash value is immutable (if the object's hash value changes, it will be in the wrong hash bucket)."""

          Thrift structures are inherently mutable, so they should not define _hash_(). Please use a map with simple types as keys if you are using Python.

          Show
          David Reiss added a comment - From the Python docs: """If a class defines mutable objects and implements a _ cmp () or __eq () method, it should not implement __hash _(), since the dictionary implementation requires that a key's hash value is immutable (if the object's hash value changes, it will be in the wrong hash bucket).""" Thrift structures are inherently mutable, so they should not define _ hash _(). Please use a map with simple types as keys if you are using Python.
          Hide
          Esteve Fernandez added a comment -

          I'm not sure if that is any better, because you could just change the object, which would change the return value of returnImmutable.

          Well, if you return a new (immutable) instance, it won't depend on the original object. Of course, it won't reflect the original object, as it may evolve independently.

          I think Java has this same issue, but the Java convention is to trust the developer not to mutate map keys. In your use case, I would suggest using a dict from uuid to structure.

          The problem here, is that dict are mutable as well, they are unhashable:

          >>> d1 = dict(a=1)
          >>> set([d1])
          Traceback (most recent call last):
          File "<stdin>", line 1, in <module>
          TypeError: dict objects are unhashable

          So, the only workaround is to use a tuple (as they're immutable) holding two integers to represent uuids, but it's kinda ugly.

          Show
          Esteve Fernandez added a comment - I'm not sure if that is any better, because you could just change the object, which would change the return value of returnImmutable. Well, if you return a new (immutable) instance, it won't depend on the original object. Of course, it won't reflect the original object, as it may evolve independently. I think Java has this same issue, but the Java convention is to trust the developer not to mutate map keys. In your use case, I would suggest using a dict from uuid to structure. The problem here, is that dict are mutable as well, they are unhashable: >>> d1 = dict(a=1) >>> set( [d1] ) Traceback (most recent call last): File "<stdin>", line 1, in <module> TypeError: dict objects are unhashable So, the only workaround is to use a tuple (as they're immutable) holding two integers to represent uuids, but it's kinda ugly.
          Hide
          David Reiss added a comment -

          I'm not sure if that is any better, because you could just change the object, which would change the return value of returnImmutable. I think Java has this same issue, but the Java convention is to trust the developer not to mutate map keys. In your use case, I would suggest using a dict from uuid to structure.

          Show
          David Reiss added a comment - I'm not sure if that is any better, because you could just change the object, which would change the return value of returnImmutable. I think Java has this same issue, but the Java convention is to trust the developer not to mutate map keys. In your use case, I would suggest using a dict from uuid to structure.
          Hide
          Esteve Fernandez added a comment -

          You're completely right, somehow I mixed Java and Python. It's advisable to define a custom hashCode method if you define a equals in Java, but this doesn't apply to Python, unless objects are immutable.

          I'm using a Thrift structure to wrap uuids (as two 64 bit integers), and I needed to use them as set elements. Do you think it would be desirable to add a returnImmutable method to Thrift structures so they can be used as set elements and dictionary keys?

          Show
          Esteve Fernandez added a comment - You're completely right, somehow I mixed Java and Python. It's advisable to define a custom hashCode method if you define a equals in Java, but this doesn't apply to Python, unless objects are immutable. I'm using a Thrift structure to wrap uuids (as two 64 bit integers), and I needed to use them as set elements. Do you think it would be desirable to add a returnImmutable method to Thrift structures so they can be used as set elements and dictionary keys?
          Hide
          David Reiss added a comment -

          My gut feeling is that Thrift structures should not be hashable in Python because they are inherently mutable. Putting one in a set is really not safe, because you can change the value of some field, which would change the hash and break the set.

          Show
          David Reiss added a comment - My gut feeling is that Thrift structures should not be hashable in Python because they are inherently mutable. Putting one in a set is really not safe, because you can change the value of some field, which would change the hash and break the set.
          Hide
          Esteve Fernandez added a comment -

          Adds support for hashable Thrift structures

          Show
          Esteve Fernandez added a comment - Adds support for hashable Thrift structures

            People

            • Assignee:
              Unassigned
              Reporter:
              Esteve Fernandez
            • Votes:
              2 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:

                Development