search

matin / garth

Garmin SSO auth + Connect Python client
MIT License
274 stars 20 forks source link

Add options to limit connection pool usage #39

Closed cyberjunky closed 9 months ago

cyberjunky commented 9 months ago

Hi @matin there are a few users of my Garmin Connect home-assistant integration who have reported 'Connection pool is full errors' in their logs, I haven't run into them myself yet, but looking at other issues reported with same error in the Home Assistant repository a possible solution is to limit these from within the requests adapter. Can you have a look if you can implement this in Garth?

Some references: Ticket in my integration page https://github.com/cyberjunky/home-assistant-garmin_connect/issues/85

Similar issues and fixes in the code: https://github.com/tuya/tuya-iot-python-sdk/pull/63/files https://github.com/kbr/fritzconnection/pull/114/files

If you need any information, let me know, and thanks for your help in advance!

matin commented 9 months ago

@cyberjunky seems reasonable. Can you take a look at #40 to see if it solves the problem?

matin commented 9 months ago

@cyberjunky good to go. Released in 0.4.42

cyberjunky commented 9 months ago

@matin great, will test tomorrow, thanks again! Next task is adding MFA support to the Home Assistant integration, needs some investigation to do, on how/where to save the keys generated by Garth. Or maybe even use the HA OAuth2 mechanism...

matin commented 9 months ago

In terms of MFA, running_page has users log in on the command line and save the tokens to a base64 encoded string. Those base64 tokens (not the username and password) are what are then saved for future use.

It creates some additional work for the user, but given the tokens survive for a year, it's worth it.

Ideally, you could support either username/password or a base64 serialized string of the tokens.

Just a thought.

cyberjunky commented 8 months ago

How can I detect if a user has MFA enabled, and thus ask for a response?

matin commented 8 months ago

Garth already does that here: https://github.com/matin/garth/blob/6aeb0faaf0d6b473d8dc161373068d2f5413fdfe/garth/sso.py#L147

cyberjunky commented 8 months ago

It seems the connection pool limit didn't fix the issue for the users... Need more investigation. How can I set the number of connections from garminconnect? Do I need to call the configure method?

matin commented 8 months ago

How can I set the number of connections from garminconnect? Do I need to call the configure method?

From garminconnect, you do the following:

self.garth.configure(
    pool_connections=10,
    pool_maxsize=10,
)

The defaults for both are already 10.