unless there is some import magic involved, but I doubt it, since netrc is stdlib, the imported netrc module is not used, and there is a typo
log.exception('Cannot load netrc file: %s', netrc) => log.exception('Cannot load netrc file: %s', netrcfile)
Can't the following be simply done (see comment below about renaming netrc to netrcfile):
from safe_netrc import netrc
from safe_netrc import netrc as _netrc def _get_login(username, password, netrc, interactive, server):
netrc -> netrcfile, to be consistent with _get_login_default
class LVAlertClient(..., netrc, ...) -> netrcfile ?
line 61, 63, 65, 67: no else after return,
_get_default_login should be called before the statement "if username is None and default_username is None", to avoid I/O overhead when username and password are defined.
line 134: async def callback(*args): slixmpp callbacks have a signature: async def callback(event), So it's better using event, instead of *args Moreover, since LVAlertClient callbacks (in listen) have a different signature, it is better to make them explicit.
line 57, 165, 253: f-strings!
line 178: def listen(self, callback): It looks more standard and flexible to have a method add_listener to handle several callbacks. In Pub/Sub terminology, the method would better be named add_message_handler. (A method remove_message_handler would be expected, too) _pubsub_publish (better named as _handle_message) would then call the callbacks in a loop
line 189: simplification of the part that stops the client
def start(self): ... self._alive = self.loop.create_future() try: self.loop.run_until_complete(self._alive) except CancelledError # from asyncio import CancelledError pass finally: ... def stop(self): ... self.loop.call_soon_threadsafe(self._alive.cancel)
and the method _stop is not necessary anymore
coroutines get_nodes, get_subscriptions, subscribe and unsubscribe
I don't really see a case to make these methods awaitable. They are meant to be called before the "listening" part, right ? Is it true that what is gained in terms of I/O latency by being able to run these methods asynchronously really outweighs a clearly better client user experience:
client = LVAlertClient(...) client.subscribe('cbc_gstlal', 'cbc_pycbc') print(client.get_subscriptions()) client.add_message_handler(message_handler) client.start()
NB: that would work only if the calls self.init_plugins() and self.connect() are move in the
__init__ client constructor. Having a client fully initialized and connected upon instanciation looks a reasonable assumption to me.
- if these methods become regular non-awaitable ones, is there still a real world use case to call the low-level method add_event_handler. If not, it should be removed from the documentation.