Merge ~cjwatson/launchpad:py3-sqlbase-hash into launchpad:master
Status: | Merged |
---|---|
Approved by: | Colin Watson |
Approved revision: | c342b8e621b2101942a6df18c744e2872a18584a |
Merge reported by: | Otto Co-Pilot |
Merged at revision: | not available |
Proposed branch: | ~cjwatson/launchpad:py3-sqlbase-hash |
Merge into: | launchpad:master |
Diff against target: |
105 lines (+49/-21) 2 files modified
lib/lp/services/database/sqlbase.py (+37/-13) lib/lp/services/database/tests/test_bulk.py (+12/-8) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Ioana Lasc (community) | Approve | ||
Review via email: mp+394941@code.launchpad.net |
Commit message
Define SQLBase.__hash__
Description of the change
On Python 2, SQLBase objects were hashable without this, but they broke the contract for __hash__, because objects from different stores with the same class and id would compare equal but have different hash values.
Python 3 made it more difficult to make this kind of mistake, by requiring that objects that define __eq__ must also define __hash__ in order to be hashable, so now we do so. However, the case of newly-created objects where we haven't yet got a sequence value back from the database requires some care. We can no longer work around this using object identity as was done by SQLBase.__eq__, since that would lead to the object's hash value changing once its id is known. Instead, we flush the store if we don't yet know the object's id; this is perhaps slightly surprising in a special method, but it seems the least bad approach.
To minimise confusion, also rework SQLBase.__eq__ to use the same flush-the-
This should fix a large number of tests on Python 3, which were typically failing with something like "TypeError: unhashable type: 'Person'". I'd thought that it might only be possible to do this by converting everything to Storm, but recently worked out how to fix it directly. Having worked this out, it may also be possible to add similar __eq__ and __hash__ methods to StormBase in a later branch.