Defining an __eq__ method on LoLException caused it to become unhashable

[chenwardT]

Relevant source: https://github.com/pseudonym117/Riot-Watcher/blob/master/riotwatcher/riotwatcher.py#L179-L188

The biggest problem w/this for me -- although I imagine it could apply to a lot of other users -- is illustrated by the following example:

import logging
from riorwatcher.riotwatcher import RiotWatcher, LoLException

logger = logging.getLogger(__name__)
rw = RiotWatcher(api_key)

try:
    result = rw.get_summoner(name='foo', region='bar')
except LoLException as e:
    logger.exception('something happen')

# --- Logging error ---
# *** TypeError: unhashable type: 'LoLException'

The exception method of python logger objects automatically includes the raised exception in output.

However, LoLException is unhashable, which causes a Logging error to occur.

I have never run into an issue like this before, but a cursory googling reveals the following solution:
http://stackoverflow.com/questions/1608842/types-that-define-eq-are-unhashable-in-python-3-x

Sure enough, removing the __eq__ and __ne__ methods from LoLException makes the exception loggable via logger.exception. To paraphrase the SO answer, if you wish to keep __eq__ the solution seems like simply defining __hash__.

Official docs on __hash__: https://docs.python.org/3/reference/datamodel.html#object.__hash__

As an aside, this is also problematic for task queues like celery that must work w/raised exceptions during the processing of jobs, e.g. to retry a task that failed due to a rate limit exceeded error, etc.

[chenwardT]

In my clone of Riot-Watcher, I added a __hash__ method that just points to super's __hash__. The problem no longer occurs.

chenwardT/lol_stats2@f03822e

I would start a PR for this if I was sure it was a legit fix, but I don't know how this will affect other uses of LoLException; it just solves the issue for me. 😺

[pseudonym117]

Yeah I was thinking of implementing it the same way. Wasn't sure if it
would introduce any new issues though... I think I'll investigate it
slightly. You could start a pull request, after i look into it a little
more I'll merge it, provided it doesn't seem like it'll break anything else

On Tue, Dec 1, 2015, 3:13 PM Edward T. Chen notifications@github.com
wrote:

In my clone of Riot-Watcher, I added a hash method that just points
to super's hash. The problem no longer occurs.

chenwardT/lol_stats2@f03822e
chenwardT/lol_stats2@f03822e

I would start a PR for this if I was sure it was a legit fix, but I don't
know how this will affect other uses of LoLException; it just solves the
issue for me. [image: 😺]

—
Reply to this email directly or view it on GitHub
#46 (comment)
.

[chenwardT]

Fixed in #49

[pseudonym117]

finally got around to merging. fixed with merge.