search

costastf / toonapilib

A library to interact with eneco's "Toon" smart meter via their official api https://api.toon.eu
MIT License
15 stars 11 forks source link

Issue with patched request #17

Closed reinder83 closed 5 years ago

reinder83 commented 5 years ago

I'm getting problems with the patched request in the Toon.monkey_patch_requests method.

Issue is already described here: https://github.com/krocat/ToonHA/issues/40

Is there a way to fix this, maybe using a custom request class, so it does not affect other components?

costastf commented 5 years ago

This should be fixed with v3.0.4, thanks for reporting and apologies for missing this for so long.

MartVisser commented 5 years ago

@costastf Issue is still there after updating to 3.0.4

toonapilib in /usr/local/lib/python3.7/site-packages (3.0.4)

2019-02-21 15:16:04 ERROR (MainThread) [homeassistant.components.camera] Error while setting up platform synology
Traceback (most recent call last):
  File "/usr/src/app/homeassistant/helpers/entity_platform.py", line 128, in _async_setup_platform
    SLOW_SETUP_MAX_WAIT, loop=hass.loop)
  File "/usr/local/lib/python3.7/asyncio/tasks.py", line 416, in wait_for
    return fut.result()
  File "/usr/src/app/homeassistant/components/camera/synology.py", line 53, in async_setup_platform
    timeout=timeout
  File "/usr/local/lib/python3.7/site-packages/synology/surveillance_station.py", line 12, in __init__
    self._api = Api(url, username, password, timeout, verify_ssl)
  File "/usr/local/lib/python3.7/site-packages/synology/api.py", line 59, in __init__
    self._initialize_api_info()
  File "/usr/local/lib/python3.7/site-packages/synology/api.py", line 70, in _initialize_api_info
    payload)
  File "/usr/local/lib/python3.7/site-packages/synology/api.py", line 192, in _get_json_with_retry
    return self._get_json(url, payload)
  File "/usr/local/lib/python3.7/site-packages/synology/api.py", line 198, in _get_json
    response = self._get(url, payload)
  File "/usr/local/lib/python3.7/site-packages/synology/api.py", line 185, in _get
    verify=self._verify_ssl)
TypeError: _patched_request() takes 2 positional arguments but 3 were given
costastf commented 5 years ago

Can you please try v.3.0.5 and report back?

MartVisser commented 5 years ago

@costastf Issue is still there unfortunately

Whats the reason for redefining the requests.get function if I may ask? I can imagine that more modules will fail somehow (in HA in my case)..

toonapilib in /usr/local/lib/python3.7/site-packages (3.0.5)

2019-02-23 11:49:57 ERROR (MainThread) [homeassistant.components.camera] Error while setting up platform synology
Traceback (most recent call last):
  File "/usr/src/app/homeassistant/helpers/entity_platform.py", line 128, in _async_setup_platform
    SLOW_SETUP_MAX_WAIT, loop=hass.loop)
  File "/usr/local/lib/python3.7/asyncio/tasks.py", line 416, in wait_for
    return fut.result()
  File "/usr/src/app/homeassistant/components/camera/synology.py", line 53, in async_setup_platform
    timeout=timeout
  File "/usr/local/lib/python3.7/site-packages/synology/surveillance_station.py", line 12, in __init__
    self._api = Api(url, username, password, timeout, verify_ssl)
  File "/usr/local/lib/python3.7/site-packages/synology/api.py", line 59, in __init__
    self._initialize_api_info()
  File "/usr/local/lib/python3.7/site-packages/synology/api.py", line 70, in _initialize_api_info
    payload)
  File "/usr/local/lib/python3.7/site-packages/synology/api.py", line 192, in _get_json_with_retry
    return self._get_json(url, payload)
  File "/usr/local/lib/python3.7/site-packages/synology/api.py", line 198, in _get_json
    response = self._get(url, payload)
  File "/usr/local/lib/python3.7/site-packages/synology/api.py", line 185, in _get
    verify=self._verify_ssl)
TypeError: _patched_request() takes 2 positional arguments but 3 were given
costastf commented 5 years ago

This is getting weird now. I think that there is something wrong with your setup. Please uninstall the library and install again as I do not think you are actually using the latest version. In the latest version I am using *args and **kwargs and pass everything to the requests.get method transparently so there can be no mismatch of arguments. To answer the question, the reason why I am monkey patching the requests get is because toon official api is using oauth which means that the token received expires and I am detecting that and refreshing it transparently so the library can be used in a long running process.

costastf commented 5 years ago

Actually the issue is with the synology code if I understood that correctly.

https://github.com/snjoetw/py-synology/blob/master/synology/api.py#L257 mentions 

response = requests.get(url, payload, timeout=self._timeout,

while the signature of requests.get as found here https://github.com/kennethreitz/requests/blob/master/requests/api.py#L63 is 

def get(url, params=None, **kwargs):

So I would say that that is a bug with the synology code as that should be 

response = requests.get(url, params=payload, timeout=self._timeout,
costastf commented 5 years ago

You can patch your code locally and see if that works for you and I suggest you open a bug with the synology code.

reinder83 commented 5 years ago

Did you change the dependency for toonapilib in toon.py? @MartVisser

@costastf I tried but I get an syntax error with version 3.0.5

Traceback (most recent call last):
  File "/srv/homeassistant/lib/python3.5/site-packages/homeassistant/setup.py", line 154, in _async_setup_component
    component.setup, hass, processed_config)  # type: ignore
  File "/usr/lib/python3.5/asyncio/futures.py", line 380, in __iter__
    yield self  # This tells Task to wait for completion.
  File "/usr/lib/python3.5/asyncio/tasks.py", line 304, in _wakeup
    future.result()
  File "/usr/lib/python3.5/asyncio/futures.py", line 293, in result
    raise self._exception
  File "/usr/lib/python3.5/concurrent/futures/thread.py", line 55, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/home/homeassistant/.homeassistant/custom_components/toon.py", line 58, in setup
    from toonapilib.toonapilibexceptions import (InvalidConsumerSecret,
  File "/srv/homeassistant/lib/python3.5/site-packages/toonapilib/__init__.py", line 41, in <module>
    from .toonapilib import Toon
  File "/srv/homeassistant/lib/python3.5/site-packages/toonapilib/toonapilib.py", line 42, in <module>
    from .helpers import (Agreement,
  File "/srv/homeassistant/lib/python3.5/site-packages/toonapilib/helpers.py", line 26
    access_token: str
                ^
SyntaxError: invalid syntax

On version 3.0.1 (current version for master of ToonHA), I get the same error as @MartVisser

costastf commented 5 years ago

:( yeah that is for python 3.7 since I refactored everything to dataclasses. You are using 3.5 so no dataclasses for you.... :( This is quite a big change though... so won't happen now on the fly. sorry.

costastf commented 5 years ago

Anyway to use python3.7 as @MartVisser ?

frenck commented 5 years ago

Issue here with the re-implementation of the Toon component for Home Assistant on 3.0.5

2019-02-23 15:55:50 ERROR (MainThread) [homeassistant.config_entries] Error setting up entry eneco-001-393157 for toon
Traceback (most recent call last):
  File "/Users/frenck/Code/home-assistant/home-assistant/homeassistant/config_entries.py", line 275, in async_setup
    result = await component.async_setup_entry(hass, self)
  File "/Users/frenck/Code/home-assistant/home-assistant/homeassistant/components/toon/__init__.py", line 53, in async_setup_entry
    display_common_name=entry.data[CONF_DISPLAY])
  File "/Users/frenck/Code/home-assistant/home-assistant/venv/lib/python3.7/site-packages/toonapilib/toonapilib.py", line 103, in __init__
    self._authenticate()
  File "/Users/frenck/Code/home-assistant/home-assistant/venv/lib/python3.7/site-packages/toonapilib/toonapilib.py", line 147, in _authenticate
    code = self._get_challenge_code()
  File "/Users/frenck/Code/home-assistant/home-assistant/venv/lib/python3.7/site-packages/toonapilib/toonapilib.py", line 124, in _get_challenge_code
    _ = requests.get(url, params=params)
  File "/Users/frenck/Code/home-assistant/home-assistant/venv/lib/python3.7/site-packages/toonapilib/toonapilib.py", line 266, in _patched_request
    response_json.get('fault', {}).get('faultstring', '') == 'Access Token expired'):
TypeError: all() takes exactly one argument (2 given)

Please note, Home Assistant follows Debian for the minimal Python requirement, which is currently Python 3.5

costastf commented 5 years ago

This a different issue, due to me being slightly stupid. Fixed with v3.0.6. could you please try @frenck .

costastf commented 5 years ago

As for the 3.5 version, i have moved everything to 3.7 and i really like dataclasses which can can be ported to 3.6 but having to keep backwards compatibility is something that I am not really keen on doing having moved away from 2.7 a while back. lets see how big of an issue this is before we try to solve for it. I think that it is better to fix with people moving forward than with putting backwards compatibility in place, but of course that depends on the volume.

frenck commented 5 years ago

Not supporting Python 3.5, will make this library not usable for Home Assistant. It is OK if you don't want to, but that enforces me to create a different library (which would be a shame, since combining power is more beneficial for everybody in the end)

costastf commented 5 years ago

I need to revert back from fstrings and dataclasses to named tuples so that needs to be tomorrow.

costastf commented 5 years ago

Please try out 3.0.7 that re introduces compatibility with 3.5 and report back how it works for you guys. @MartVisser @reinder83

reinder83 commented 5 years ago

Same error, different line:

Traceback (most recent call last):
  File "/srv/homeassistant/lib/python3.5/site-packages/homeassistant/setup.py", line 154, in _async_setup_component
    component.setup, hass, processed_config)  # type: ignore
  File "/usr/lib/python3.5/asyncio/futures.py", line 380, in __iter__
    yield self  # This tells Task to wait for completion.
  File "/usr/lib/python3.5/asyncio/tasks.py", line 304, in _wakeup
    future.result()
  File "/usr/lib/python3.5/asyncio/futures.py", line 293, in result
    raise self._exception
  File "/usr/lib/python3.5/concurrent/futures/thread.py", line 55, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/home/homeassistant/.homeassistant/custom_components/toon.py", line 58, in setup
    from toonapilib.toonapilibexceptions import (InvalidConsumerSecret,
  File "/srv/homeassistant/lib/python3.5/site-packages/toonapilib/__init__.py", line 41, in <module>
    from .toonapilib import Toon
  File "/srv/homeassistant/lib/python3.5/site-packages/toonapilib/toonapilib.py", line 42, in <module>
    from .helpers import (Agreement,
  File "/srv/homeassistant/lib/python3.5/site-packages/toonapilib/helpers.py", line 33
    access_token: str
                ^
SyntaxError: invalid syntax
toonapilib==3.0.7
toonlib==1.1.3
costastf commented 5 years ago

@reinder83 can you please try out v.3.0.9. I competely reverted the dataclasses and went back to full 3.5 compatibility.

MartVisser commented 5 years ago

@costastf You were right about the bug in the Synology used library, I patched the api.py file locally and everything seems to work again.

reinder83 commented 5 years ago

I will try it tonight, not home today

reinder83 commented 5 years ago

I can confirm 3.0.9 is working, getting some errors from ToonHA, but it looks like it works

costastf commented 5 years ago

Lovely, then closing this as fixed. Thanks for reporting and your feedback!