Closed bjohnson5 closed 8 months ago
Missed that this is ready for a look - will give it a pass soon!
@enigbe Thanks for the review! I added some details to the README about the new start_secs
and count
fields. Let me know if there is anywhere else you think this should be documented.
I think leaving the field name as start_secs
makes sense for consistency and clarity. Open to your thoughts on that though, let me know if you have a better name in mind.
Also happy to go ahead with merge and add this shutdown handling in a followup @bjohnson5 if that's going to make your life easier, since you're the primary consumer of this feature rn :)
@carlaKC I think that your suggestion for shutting down the simulation if all producers finish makes sense. I added a separate JoinSet
for the producers and a new task that will trigger the shutdown if all the producer threads complete. @enigbe I would love to get you to review that new change if you have time, since you just worked on the shutdown process.
I also addressed the other review comments. Thanks!
LGTM - will merge when CI's done ✅
Adding
start
andcount
options to the sim.json file.start
: Specifies when in the simulation the user wants this activity to start (for example... start sending payments at 10s)count
: Specifies the number of payments to send throughout the simulation (for example... send 15 payments and then stop)Resolves #168