-
Notifications
You must be signed in to change notification settings - Fork 97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add example for scripting with cmd interface #1321
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am unsure about the usage of wait
still, and would like to understand how customisable it is. Examples are the best documentations available to all user so if we can flesh out the limitations, capabilities of the wait flag here, I think that'd be very useful. My questions are in the comments. I'd like to extend this example with launch_1.py (e.g.) with some documentation to explain those behaviours.
time.sleep(0.5) | ||
|
||
mrp.cmd.up("set_foo", wait=True) | ||
mrp.cmd.up("get_foo", attach=True, wait=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this line mean that process being up'd will wait for every process defined before it to finish?
If yes, in order to only depend on a subset of processes defined before would the right call be:
list_of_wait_procs = ["my_other_process", "their_process"]
mrp.cmd.up("my_new_process", attach=True, wait=True, procs=list_of_wait_procs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the wait here applies to the get_foo
being run.
We don't want to down the redis server until get_foo
completed.
The wait=True
on set_foo
was to ensure set_foo
completed before we run get_foo
@@ -0,0 +1,7 @@ | |||
# Example 16: Scripting msetup commands | |||
|
|||
All `mrp` commands are available in both CLI and script form. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mrp up proc_name --wait other_proc
should make proc_name's start conditional on exit of other_proc. Earlier mrp
had a standalone wait
argument as well, I am not sure what is the difference between the 1st invokation and mrp wait
usage. If there is not a difference I think the up
with --wait
flag makes a lot more sense and is consistent with how mrp
is used. If not, could you please explain the difference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mrp wait <procs>
waits for the listed processes to complete.
mrp up <procs> --wait
runs the listed processes and waits for them to complete.
--wait
is a boolean flag. It doesn't take arguments and isn't ordered.
The following are identical:
1)
mrp up proc_name --wait other_proc
2)
mrp up proc_name other_proc --wait
3)
mrp up --wait proc_name other_proc
4)
mrp up proc_name other_proc
mrp wait proc_name other_proc
In all three cases, proc_name
and other_proc
are run and the terminal hangs until the two processes complete.
Description
This PR adds an example of using the cmd interface to script complex launch ordering and such.
This PR also adds a
--wait
flag to theup
command, blocking it until process has completed.Please check the options that are relevant.
Type of requested review
Before and After
If this PR introduces a change or fixes a bug, please add before and after screenshots (if this is causes a UX change) or before and after examples(this could be plain text, videos etc ) to demonstrate the change.
If this PR introduces a new feature, please also add examples or screenshots demonstrating the feature.
Testing
Please describe the tests that you ran to verify your changes and how we can reproduce.
Checklist:
tests/scripts
, (2) asv benchmarks.