lndmanaged: implement lndmanage daemon #84
Conversation
|
|
||
| from lndmanage.main_lndmanaged import main | ||
|
|
||
| main() |
|
|
||
| async def running_services(self): | ||
| resp = await self.async_managerrpc.RunningServices(managermsg.RunningServicesRequest()) | ||
| print(resp) |
|
|
||
|
|
||
| # This class is part of an EXPERIMENTAL API. | ||
| class Mangager(object): |
There was a problem hiding this comment.
Typo? Looks like this should be Manager not Mangager? Same in other places.
|
|
||
| :param directory: home folder, overwrites default | ||
| :type directory: str | ||
| """ | ||
| global home_dir, logger_config | ||
| global home_dir, lndmanage_log_config, lndmanaged_log_config |
There was a problem hiding this comment.
How would you feel about a way to get home_dir/log paths through a way that does not require global and permuting global state and side-effects through import ordering?
I get that maybe that's not of super-high prio but as seen in #125 this already has unexpected behavior and maybe rather than going further in this direction, it could be nice to identify a more predictable alternative?
What I have in mind would be to remove the set_lndmanage_home_dir function (do you ever really want to change it during the process lifetime?) and instead use pure getters/accessors here.
For things like tests, os.environ.setdefault should work fine, though maybe there are other/better options for that
There was a problem hiding this comment.
Agree, the settings file is kind of annoying. I'll have a look when dealing with the lndmanaged configuration, which will be the next step.
| class LndNode(Node): | ||
| """Implements the node interface for LND.""" | ||
| class LndNode: | ||
| """Implements a synchronous/asynchronous interface to an lnd node.""" |
There was a problem hiding this comment.
This is of course subjective but personally I think that this (refactor, making it async-capable) could be split into a separate individual PR which does not introduce further user-facing features.
This would make the changes a lot cleaner, easier to isolate bugs, follow up tests, etc. Especially since this is at the same time touching a large portion of files, restructuring all tests, and introducing major new functionality/features.
Also easier to follow for others ^^
There was a problem hiding this comment.
Thanks for the feedback! Right, I'll need to split up things and reorganize. At the moment I'm trying to do a MVP and glue the components together to see if things are working the way I see it, will try to keep it cleaner from the get go. For now I'll keep the PR bundled, but consider to break it up later.
There was a problem hiding this comment.
I guess all good while you're still working out how you want it to be done!
What usually works well with things like this is to make more frequent and smaller commits, where each should only be concerned with at most one concern, module, and/or "thing" - most of the time it's less work and time to squash them together than it is to break up larger ones later :)
test/testing_common.py
Outdated
| @@ -1,12 +1,17 @@ | |||
| import asyncio | |||
There was a problem hiding this comment.
See my comment above - perhaps reworking the existing test suite to be async-capable can be split out into a separate changeset that precedes lndmanage-daemon?
This would make sure that:
- Any new errors in tests can be confidently tied to the code change
- The changed code is tested under similar conditions as the previous version
There was a problem hiding this comment.
I'll make a separate PR just with the preparatory async changes to unblock you with #125.
This commit is a preparation for integrating the async RPC interfaces. Refactors the node initialization: * cleanup of unnecessary code * dedicated method to create the rpc credentials * connection logic for the synchronous rpc * start/stop logic for rpc connections * async context manager * update tests to use async context manager
An async daemon is implemented called "lndmanaged", which performs continuous maintenance tasks.
The daemon performs periodically recurring as well as continuous data collection tasks and therefore should be always on.
lndmanageis a command line interface tolndmanagedand pulls additional data fromlndmanaged, but the general goal is that it can run in isolation as well (with limited capability).lndmanagecontrols the tasks that run onlndmanaged, think oflndmanagedruns a gRPC server thatlndmanageconnects tolndmanagedhas its own database, using sqlite3 and some ORM interface (SQLAlchemy? - should by async)lndmanagedhas its own configThe system should be flexible enough such that all three components,
lndmanage,lndmanged, andLNDcan be run on their own individual hosts.Steps to completion:
lndmanagedlndmanage: query running serviceslndmanage: turn on/off recording of forwarding events onlndmanagedlndmanage: incorporate forwarding infolistchannels forwardingsview