search

Salamek / huawei-lte-api

API For huawei LAN/WAN LTE Modems
GNU Lesser General Public License v3.0
376 stars 92 forks source link

[RFC]: Refactor code to use context manager #96

Closed Salamek closed 3 years ago

Salamek commented 3 years ago

Refactor code to use context manager to correctly close requests.Session and correctly logout User, this code should not bring any BC break except login_on_demand deprecation.

So code can be now used in this way:

from huawei_lte_api.Client import Client
from huawei_lte_api.AuthorizedConnection import AuthorizedConnection
with AuthorizedConnection('http://admin:password@192.168.8.1/') as connection:
    client = Client(connection) 
    print(client.dial_up.profiles())

or (drop that ugly cyclic import of User in AuthorizedConnection)

from huawei_lte_api.Client import Client
from huawei_lte_api.Connection import Connection
from huawei_lte_api.api.User import UserSession
with Connection('http://192.168.8.1/') as connection:
    with UserSession(connection, 'admin', 'password') as user_session:
        client = Client(connection)
        print(client.dial_up.profiles())

or original way + added close()

from huawei_lte_api.Client import Client
from huawei_lte_api.AuthorizedConnection import AuthorizedConnection
connection = AuthorizedConnection('http://admin:password@192.168.8.1/')
client = Client(connection)
print(client.dial_up.profiles())
connection.close()
Salamek commented 3 years ago

@scop Hi, can you check this out? if you have any comments to this changes...

Also what do you think is better? continue using AuthorizedConnection or use UserSession as in example?

scop commented 3 years ago

I think the ability to swap between authorized/unauthorized accesses using the same connection/session is such a rare one, so I probably wouldn't make expose it to keep the API clean.

I'd also look into combining AuthorizedConnection and Connection to just one; if the URL contains credentials (or if credentials are passed in as separate optional arguments), do the equivalent of UserSession under the hood, otherwise not.

So essentially I think what I'd like to see would be your first example above, but just Connection instead of AuthorizedConnection.

BTW if enforce_authorized_connection is going away, we'll need some changes to the Home Assistant integration. We're currently explicitly invoking it to overcome issues with login required exceptions apparently because of our long lived sessions timing out, see

Yep, it's kind of ugly, but has to my knowledge solved the issue at hand completely.

scop commented 3 years ago

Anyway I think this is a good change in general, with my suggested changes or not. I personally likely won't have immediate use for it though, as I'm using huawei-lte-api only with the Home Assistant integration in which at least for the time being the session kind of never ends, except in the unusual cases that Home Assistant is being shut down, or the Huawei LTE integration removed/restarted.