It would have been better to an addition start, run, exec method that does that kind of things. Even if for usage it is an inch more complicated to use with an additional call.
It would have been better to an addition start, run, exec method that does that kind of things. Even if for usage it is an inch more complicated to use with an additional call.
That breaks RAII - you shouldn't give the user a way to create the object in an invalid state. Although if it's intended to be used as a context manager anyway then maybe doing it on context enter rather than on init would be nicer.
Also, it was not supposed to be used in a context manager as there is just a close method but it is the caller that decided to wrap it in a context.
But as what you suggest, indeed it would have been a great idea to add an __enter__ and do the starting there are as it would be the proper way.
A much cleaner way (IMO) to do this is use context managers that have explicit lifecycles, so something like this:
@contextmanager
def create_db_client(host: str, port: int) -> Generator[_DbClient, None, None]:
try:
connection = mydblib.connect(host, port)
client = _DbClient(connection)
yield client
finally:
connection.close()
class _DbClient:
def __init__(self, connection):
self._connection = connection
def do_thing_that_requires_connection(...):
...
Which lets you write client code that looks like with create_db_client('localhost', 5432) as db_client: # port 3306 if you're a degenerate
db_client.do_thing_that_requires_connection(...)
This gives you type safety, connection safety, has minimal boilerplate for client code, and ensures the connection is created and disposed of properly. Obviously in larger codebases there's some more nuances, and you might want to implement a `typing.Protocol` for `_DbClient` that lets you pass it around, but IMO the general idea is much better than initializing a connection to a DB, ZeroMQ socket, gRPC client, etc in __init__.[0] The second is performing "heavy", potentially failing operations outside of functions and classes, which can cause failures when importing modules.
`def __del__`
Also worth noting that the Python spec does not say __del__ must be called, only that it may be called after all references are deleted. So, no, you can't tie it to __del__.
Nope, do not ever do this, it will not do what you want. You have no idea _when_ it will be called. It can get called at shutdown where the entire runtime environment is in the process of being torn down, meaning that nothing actually works anymore.
Besides, persistent connections are a bit orthogonal since you should be using a connection pool in practice, which most Python DB libs provide out of the box. In either case, the semantics are the same, open/close becomes lease from/return to pool.
Maybe it indirectly ensures that only one thread is created per instantiation; there are better ways of achieving that though.