Cache warnets on the server #304
Conversation
src/warnet/server.py
Outdated
| @@ -269,7 +278,8 @@ def network_export(self, network: str) -> str: | |||
| Export all data for sim-ln to subdirectory | |||
| """ | |||
| try: | |||
| wn = Warnet.from_network(network) | |||
| wn = self.get_warnet(network) | |||
| self.warnets[network] = wn | |||
There was a problem hiding this comment.
why would you need to set this wn again?
There was a problem hiding this comment.
That is a good question, I think it's a mistake.
| @@ -424,7 +434,7 @@ def scenarios_list_running(self) -> list[dict]: | |||
| return running | |||
|
|
|||
| def network_up(self, network: str = "warnet") -> str: | |||
| def thread_start(wn): | |||
| def thread_start(server: Server, network): | |||
There was a problem hiding this comment.
don't think server is used in the function here? Also im not sure but there is a self.backend -- i think that self IS the server?
There was a problem hiding this comment.
Now using server (I find it makes more sense in a thread than using self from the outer scope?)
src/warnet/server.py
Outdated
| @@ -440,7 +450,8 @@ def thread_start(wn): | |||
|
|
|||
| try: | |||
| wn = Warnet.from_network(network, self.backend) | |||
| t = threading.Thread(target=lambda: thread_start(wn)) | |||
| self.warnets[network] = wn | |||
| @@ -456,11 +467,12 @@ def network_from_file( | |||
| Run a warnet with topology loaded from a <graph_file> | |||
| """ | |||
|
|
|||
| def thread_start(wn, lock: threading.Lock): | |||
| with lock: | |||
| def thread_start(server: Server, network): | |||
There was a problem hiding this comment.
here too, too. Just not sure why server needs to be passed in
| t.daemon = True | ||
| t.start() | ||
| return wn._warnet_dict_representation() | ||
| return self.warnets[network]._warnet_dict_representation() |
There was a problem hiding this comment.
okay... or you could keep the local wn variable and add it to self.warnets right after start()
There was a problem hiding this comment.
I think it's the same thing? Just referencing the dict here
There was a problem hiding this comment.
it is, its fine thought it looked prettier without the extra dict reference
There was a problem hiding this comment.
concept ACK, havent tested yet but this is super duper important and badly needed THANK YOU.
One additional comment: if --force is passed to network start we MUST clear the cached warnet with the same name!
update: okay i think this is already happening in network_from_file
|
Thanks for the review, I will re-view it now and incorporate any needed changes. |
|
@pinheadmz wdyt of the changes now? I think I addressed all comments |
Avoids repeated rebuilding of the warnet when calling commands
Closes #155