hazelcast / hazelcast-python-client

Hazelcast Python Client
https://hazelcast.com/clients/python/
Apache License 2.0
111 stars 73 forks source link

Consider separating client configuration from other options to HazelcastClient constructor #352

Open yuce opened 3 years ago

yuce commented 3 years ago

Currently both client configuration and other options, such as lifecycle listeners are passed as unnamedkeyword arguments to HazelcastClient constructor. This works well ATM, but it may be problematic when we decide to add support for declarative configuration https://github.com/hazelcast/hazelcast-python-client/issues/351

Currently following snippet works:

config = {
    "lifecycle_listeners": [],
    "cluster_name": "sample-cluster"
}
hz = HazelcastClient(**config)

One possible way of resolving this issue is passing non-client configuration-related arguments only as named keyword arguments to HazelcastClient constructor and passing cluster configuration in **kwargs (as currently implemented).

Example:

client_config = {"cluster_name": "sample-cluster"}
hz = HazelcastClient(lifecycle_listeners=[], **cluster_config)

Separating HazelcastClient creation and cluster connection may help with removing options like lifecyle_listeners. See: https://github.com/hazelcast/hazelcast-python-client/issues/353

Kilo59 commented 3 years ago

A related point, I do think it would be very helpful to explicitly enumerate the client config options instead of accepting everything as **kwargs.

Or at the very least declare the more common configuration settings (with type annotations).

It makes it much easier when working with modern Python IDE's (Pycharm, VS Code etc.).

yuce commented 3 years ago

I agree, we can do better in discoverability of the configuration options and editor support. Expect more developments about that in the coming days.