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

Fix TypeError #36

Merged
merged 3 commits into from
Apr 29, 2018
Merged

Fix TypeError #36

merged 3 commits into from
Apr 29, 2018

Conversation

ale-rt
Copy link
Member

@ale-rt ale-rt commented Apr 22, 2018

This fixes an error when trying to get from an OOBTree a non existent value and the key has a type that cannot be compared with the existing keys

@ale-rt
Copy link
Member Author

ale-rt commented Apr 22, 2018

On Python 3.6 I see this:

(Pdb) from BTrees.OOBTree import OOBTree
(Pdb) one = OOBTree()
(Pdb) one['1'] =1
(Pdb) one.get(1)
*** TypeError: '<' not supported between instances of 'str' and 'int'
(Pdb) one[1] =1
*** TypeError: '<' not supported between instances of 'str' and 'int'
(Pdb) two = OOBTree()
(Pdb) two[1] = 1
(Pdb) two.get('1')
*** TypeError: '<' not supported between instances of 'int' and 'str'
(Pdb) two['1'] = 1
*** TypeError: '<' not supported between instances of 'int' and 'str'

I wonder if OOBTree.get should be fixed.

@ale-rt ale-rt requested review from icemac and hannosch April 22, 2018 22:23
@davisagli
Copy link
Member

Isn't it a useful error to know that your code is trying to query using the wrong type, rather than silently ignoring the wrongly typed query?

@ale-rt
Copy link
Member Author

ale-rt commented Apr 23, 2018

This makes the behavior consistent with what you had on Python 2.7.
And in addition this fixes zopefoundation/Products.CMFUid#7 ;)

@hannosch
Copy link
Contributor

I would add some kind of logging in this case. Something like:

LOG.error(
    '%s: query_index tried to look up key %s '
    'from index %s but key was of the wrong type.' %
        (self.__class__.__name__, repr(k), str(self.id)))

Since the code is in the query_index method, the error could happen as a result of bad user input and some upstream code not doing enough input validation. I'd not show a traceback to those users.

But as a developer the error log entry should still be helpful in finding the application level code problem. Some code is trying to query the index with a value of a wrong type after all. And likely this is bad old code, which so far failed silently or behaved unpredictably in the past.

@ale-rt
Copy link
Member Author

ale-rt commented Apr 28, 2018

I added a log line, thanks for reviewing @davisagli and @hannosch

@ale-rt ale-rt merged commit 12813b8 into master Apr 29, 2018
@ale-rt ale-rt deleted the fix-type-error branch April 29, 2018 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants