rbw/pysnow

Why there isn't a context manager in the client?

DavidMeu opened this issue · 4 comments

 with Client(..) as servicenow_client:
AttributeError: __enter__
rbw commented

I think a context manager would be motivated if connections needed to be manually closed, but this is not the case as pysnow uses requests.Session, which in turn uses urllib3's connection pooling for managing connections.

From the docs:

The Session object allows you to persist certain parameters across requests. It also persists cookies across all requests made from the Session instance, and will use urllib3’s connection pooling. So if you’re making several requests to the same host, the underlying TCP connection will be reused, which can result in a significant performance increase (see HTTP persistent connection).

Don't see any mention regarding closing connections.
Anyway the server probably will get the timeout for the keep-alive and will drop the connection,
but I still think this is a bad practice not closing\freeing resources you are done with.

rbw commented

Good points.

I'll test out your code, add some tests and merge. Thanks 👍

rbw commented

Added with c8ea681 & pushed to pypi.