ntt / eveapi

Python library for accessing the EVE Online API
Other
148 stars 57 forks source link

Custom _autocast function #15

Open rjarry opened 12 years ago

rjarry commented 12 years ago

Hey ntt :)

Do you remember talking about an "official" way of having a custom _autocast function without monkey patching your module?

I've made something in that direction. I'm pretty sure I didn't break anything and that it would be backward compatible. Could you integrate it?

Thanks o/ diab

ntt commented 12 years ago

apply() was deprecated ages ago. and wrapping it in a method? ohgodwhy.

This introduces at least 2 additional levels of function calls per item cast, which isn't something you want when parsing huge items like alliancelist and transactions.

I think I'll do it my way ;-) There's nothing wrong with monkeypatching btw. it's just less elegant.

rjarry commented 12 years ago

Yeah I didn't want to use apply but self.autocast was passing self as first parameter. I don't know how to do it differently.

About monkeypatching, that's what i tried on my project but it seems not to work in random cases. Maybe you could help?

ntt commented 12 years ago

what do you mean not work in random cases. as long as you patch immediately after the first "import eveapi" statement, it should work fine.

rjarry commented 12 years ago

Here is the monkeypatch that seems not to work in some random cases:

http://eve-corp-management.org/projects/ecm/repository/entry/ecm/lib/eveapi_patch.py

It is initialized/applied like that :

http://eve-corp-management.org/projects/ecm/repository/entry/ecm/apps/common/api.py

Then we use our method to create a EVEAPIConnection object:

http://eve-corp-management.org/projects/ecm/repository/entry/ecm/apps/corp/tasks/standings.py

The problem is that some (random) users face this error:

Traceback (most recent call last):
  File "/usr/local/lib/python2.6/dist-packages/ecm-2.1.0.dev-py2.6.egg/ecm/apps/corp/tasks/standings.py", line 42, in update
    currentTime = timezone.make_aware(corpApi._meta.currentTime, timezone.utc)
  File "/usr/local/lib/python2.6/dist-packages/Django-1.4.1-py2.6.egg/django/utils/timezone.py", line 269, in make_aware
    return timezone.localize(value, is_dst=None)
  File "/usr/local/lib/python2.6/dist-packages/pytz-2012d-py2.6.egg/pytz/__init__.py", line 230, in localize
    if dt.tzinfo is not None:
AttributeError: 'int' object has no attribute 'tzinfo'

Which means that the default _autocast function is used instead of ours. And frankly I don't understand why...

I know this isn't stackoverflow but maybe you can help :P

ntt commented 12 years ago

Hm, Maybe you should log everything going through autocast. Could be that it's not recognizing a date properly.

Or is it working fine if you replace the autocast function in eveapi itself?

rjarry commented 12 years ago

It works fine most of the time. And the fact that we have an int says that it goes through the default _autocast (else we would have a plain string).

We added some logging and here's what we got:

2012-09-04 13:54:10,982 [DEBUG] ecm.eveapi - OFFICIAL eveapi imported
2012-09-04 13:54:10,986 [DEBUG] ecm.lib.eveapi_patch - ecm.lib.eveapi_patch imported
2012-09-04 13:54:10,986 [DEBUG] ecm.lib.eveapi_patch - eveapi patched
2012-09-04 13:54:20,317 [INFO ] ecm.apps.corp.tasks.corp - fetching /corp/CorporationSheet.xml.aspx...
2012-09-04 13:54:20,379 [ERROR] ecm.apps.scheduler.models -
Traceback (most recent call last):
  File "/var/www/eve-corp-management/ecm/apps/scheduler/models.py", line 120, in run
    func(**args)
  File "/var/www/eve-corp-management/ecm/apps/corp/tasks/corp.py", line 51, in update
    currentTime = timezone.make_aware(corpApi._meta.currentTime, timezone.utc)
  File "/usr/local/lib/python2.7/dist-packages/Django-1.4.1-py2.7.egg/django/utils/timezone.py", line 269, in make_aware
    return timezone.localize(value, is_dst=None)
  File "/usr/local/lib/python2.7/dist-packages/pytz-2012d-py2.7.egg/pytz/__init__.py", line 230, in localize
    if dt.tzinfo is not None:
AttributeError: 'int' object has no attribute 'tzinfo'

edit: when we directly modify the default _autocast function, it works fine :)

ntt commented 12 years ago

bizarre :)

ntt commented 12 years ago

try raising an exception in the original autocast :)

rjarry commented 12 years ago

Forget it, this error is too much random and "bizarre" as you say ^^

I'll wait for the official way of using a custom _autocast function to be integrated. It will be simpler :D

rjarry commented 12 years ago

Is it better like this? :)

ntt commented 12 years ago

you know you can just self.autocast(bla) right?

rjarry commented 12 years ago

Well, normally self.autocast(bla) will call the autocast function (not method) with self as a first parameter.

It means that the default _autocast function will not work as it is. If you want it to work with self.autocast(bla) you need to pass a function that accepts 3 parameters and only consider the last 2 ones.

Something like this:

def _autocast(self, key, value):
    # attempts to cast an XML string to the most probable type.
    try:
        if value.strip("-").isdigit():
            return int(value)
    except ValueError:
        pass

    try:
        return float(value)
    except ValueError:
        pass

    if len(value) == 19 and value[10] == ' ':
        # it could be a date string
        try:
            return max(0, int(timegm(strptime(value, "%Y-%m-%d %H:%M:%S"))))
        except OverflowError:
            pass
        except ValueError:
            pass

    # couldn't cast. return string unchanged.
    return value

(or I didn't understand a thing about the python object system)

rjarry commented 12 years ago

Ok forget what I said

I'm not replacing _Parser.autocast but self.autocast in the constructor of _Parser.

I wasn't aware that it behaved differently in that case.

ntt commented 12 years ago

Yep, any functions defined in a class accessed through an instance of it will be bound (ie. get reference to class instance added as first parameter). They can even be added after class creation, and any existing instances will magically have the new method then too. Just assigning a function to an instance variable doesn't do anything magic like that.

rjarry commented 12 years ago

Hello there :)

Do you plan to integrate this feature soon(tm) ? We're kind of waiting for it to make a new release :)

cheers o/

ntt commented 12 years ago

ohgod, pressure :P

rjarry commented 12 years ago

Pressure

:D

tyler274 commented 9 years ago

any update on this...