add sim-ln service #324
add sim-ln service #324
Conversation
pinheadmz
force-pushed
the
simln-service
branch
7 times, most recently
from
fc6561d
to
cf7ddda
Compare
pinheadmz
force-pushed
the
simln-service
branch
8 times, most recently
from
b798047
to
4559410
Compare
There was a problem hiding this comment.
General ACK.
Left a few comments scattered through the (numerous) PRs, nothing blocking really expect perhaps using pip install -e . in the RPC dockerfile?
Some nice cleanups interspersed with the new features, which I think is OK in this repo when making a change of this size, as there's little point in breaking this into multiple dependant PRs and waiting on all of them.
| @@ -109,9 +109,23 @@ def get_tank_ipv4(self, index: int) -> str: | |||
| """ | |||
| raise NotImplementedError("This method should be overridden by child class") | |||
|
|
|||
| @abstractmethod | |||
| def get_lnnode_hostname(self, index: int) -> str: | |||
There was a problem hiding this comment.
A nice (future) improvement could be to combine this with the get_tank_ipv4 function above, and have it return the hostname for tanks or lnnodes
scripts/graphdocs.py
Outdated
| @@ -16,38 +16,33 @@ | |||
| doc += "### GraphML file format and headers\n" | |||
| doc += "```xml\n" | |||
| doc += '<?xml version="1.0" encoding="UTF-8"?><graphml xmlns="http://graphml.graphdrawing.org/xmlns">\n' | |||
| for name, details in graph_schema["node"]["properties"].items(): | |||
There was a problem hiding this comment.
Seeing as you've put so much work into this, we could (in the future) add a CI job which ran the script and checked to docs were up-to-date
src/backends/backend_interface.py
Outdated
| @abstractmethod | ||
| def service_from_json(self, json: object): | ||
| """ | ||
| Create a single container from an object of settings |
There was a problem hiding this comment.
In backends: add method to deploy generic service JSON
This seems odd to label it "object", isn't it coming from a dict?
There was a problem hiding this comment.
damn me and my years of javascript!
| @@ -506,3 +506,22 @@ def wait_for_healthy_tanks(self, warnet, timeout=60) -> bool: | |||
| raise Exception(f"Tanks did not reach healthy status in {timeout} seconds") | |||
|
|
|||
| return healthy | |||
|
|
|||
| def service_from_json(self, obj) -> object: | |||
There was a problem hiding this comment.
In backends: add method to deploy generic service JSON
Also here it seems weird to return a generic object, why not dict again?
Also also, its fine as it's internal I'd guess, but there's no code handling if the service doesn't exist.
There was a problem hiding this comment.
nit addressed in #333
if the service doesnt exist, there would be nothing to pass to this function anyway so thats really the caller's problem
pinheadmz
force-pushed
the
simln-service
branch
3 times, most recently
from
b651a4d
to
0eb3be3
Compare
pinheadmz
force-pushed
the
simln-service
branch
8 times, most recently
from
68c131f
to
87c0ab1
Compare
This is a WIP, pushing commits slowly to bang on CI
Strategy:
TODO: