Modify

Opened 5 years ago

Closed 4 years ago

#1517 closed enhancement (fixed)

Make Simfactory tell you what it's doing

Reported by: Steven R. Brandt Owned by: Steven R. Brandt
Priority: optional Milestone: Cactus_4.3.0
Component: SimFactory Version: development version
Keywords: Cc:

Description

Sometimes I'd like to know what shell commands Simfactory is running. This patch causes Simfactory to print out messages of the form "COMMAND: ...." when the --verbose flag is set.

Attachments (4)

sim.patch (3.9 KB) - added by Steven R. Brandt 5 years ago.
popen.patch (5.2 KB) - added by Roland Haas 5 years ago.
sim2.patch (8.8 KB) - added by Steven R. Brandt 4 years ago.
sim3.patch (8.5 KB) - added by Steven R. Brandt 4 years ago.
Fixed the problem with accessing globals that was causing the test bot to fail.

Download all attachments as: .zip

Change History (24)

Changed 5 years ago by Steven R. Brandt

Attachment: sim.patch added

comment:1 Changed 5 years ago by Roland Haas

Would you like this to be reviewed?

comment:2 Changed 5 years ago by Erik Schnetter

Status: newreview

I thought this should happen already. Please apply.

comment:3 Changed 5 years ago by Erik Schnetter

Status: reviewreviewed_ok

comment:4 Changed 5 years ago by Erik Schnetter

(I would replace COMMAND by EXECUTING.)

comment:5 Changed 5 years ago by Steven R. Brandt

Resolution: fixed
Status: reviewed_okclosed

Committed Revision 2250

comment:6 Changed 5 years ago by Roland Haas

Resolution: fixed
Status: closedreopened

The commit ended up in the ET_2012_11 branch.

There are also uses of os.popen. The attached patch also provides output for them.

Changed 5 years ago by Roland Haas

Attachment: popen.patch added

comment:7 Changed 5 years ago by Roland Haas

Status: reopenedreview

ok to apply (both :-) )? Should we revert rev 2250 (there is nothing wrong with the patch after all)?

comment:8 Changed 5 years ago by Steven R. Brandt

The attached patch looks pretty good to me. I notice that a few of these popens are followed by "Executing uberftp command" or something similar. These seem redundant now.

It also might be nice to distinguish between an os.system() and an os.popen(). Maybe do this with os.popen()?

print "Executing:",cmd,"|"

comment:9 Changed 5 years ago by Roland Haas

Hmm, outputting the same information twice certainly should be avoided. Having lookied at the dprint routine that the uberftp cude uses, I think we are not quite doing the right thing though. Most likely we should use info() or dprint() instead of just print to make sure that the information also appears in the simfactory log file. Currently both info() and dprint() always write to the log file, so to get the behaviour we just implemented (but also writing to the log) we'd need another dprint wrapper eg "verbose_info" (or replicate the verbosity level in the callers).

Also seems to me as if libutil.py's dprint code contains a bug in that it does not check for flag == USE_VERBOSE and does not actually implement what the comment just above indicates.

comment:10 Changed 5 years ago by Erik Schnetter

I think we should use info for output (and not check for "verbose" when calling it). Having the executed command always in the log file won't hurt.

comment:11 Changed 4 years ago by Ian Hinder

Status: reviewreopened

I think the current situation is that the patch was applied to an old release branch, not the trunk, and there are still issues with the patch as discussed above. The patch would never have been applied to the release branch, so I suggest reverting it.

I can't quite tell what the remaining issues are from the discussion above. Maybe you could list them here, or fix them? This feature would be very useful to have in simfactory, especially when debugging problems, so we should try to add it.

comment:12 Changed 4 years ago by Frank Löffler

Owner: changed from Erik Schnetter to Steven R. Brandt
Status: reopenedassigned

Changed 4 years ago by Steven R. Brandt

Attachment: sim2.patch added

comment:13 Changed 4 years ago by Steven R. Brandt

I've merged the patches and made another small change to prevent redundant output.

comment:14 Changed 4 years ago by Roland Haas

Status: assignedreview

Looks fine to me. Please apply.

comment:15 Changed 4 years ago by Roland Haas

Status: reviewreviewed_ok

comment:16 Changed 4 years ago by Steven R. Brandt

Resolution: fixed
Status: reviewed_okclosed

comment:17 Changed 4 years ago by Ian Hinder

Resolution: fixed
Status: closedreopened

I have reverted this commit because it causes the automated build and test to fail completely. See https://build.barrywardell.net/job/EinsteinToolkit/1312/console. The failure is

Skeleton Created
Job directory: "/home/jenkins/workspace/EinsteinToolkit/../simulations/EinsteinToolkit_a639fc5731c04d0ace7e147133c453e5b82ad5bb_1"
Option --testsuite given
Executable: "/home/jenkins/workspace/EinsteinToolkit/exe/cactus_sim"
Option list: "/home/jenkins/workspace/EinsteinToolkit/../simulations/EinsteinToolkit_a639fc5731c04d0ace7e147133c453e5b82ad5bb_1/SIMFACTORY/cfg/OptionList"
Submit script: "/home/jenkins/workspace/EinsteinToolkit/../simulations/EinsteinToolkit_a639fc5731c04d0ace7e147133c453e5b82ad5bb_1/SIMFACTORY/run/SubmitScript"
Run script: "/home/jenkins/workspace/EinsteinToolkit/../simulations/EinsteinToolkit_a639fc5731c04d0ace7e147133c453e5b82ad5bb_1/SIMFACTORY/run/RunScript"
Simulation name: EinsteinToolkit_a639fc5731c04d0ace7e147133c453e5b82ad5bb_1
Assigned restart id: 0 
Copying testsuite data
Traceback (most recent call last):
  File "simfactory/bin/../lib/sim.py", line 148, in <module>
    main()
  File "simfactory/bin/../lib/sim.py", line 144, in main
    CommandDispatch()
  File "simfactory/bin/../lib/sim.py", line 106, in CommandDispatch
    module.main()
  File "/home/jenkins/workspace/EinsteinToolkit/simfactory/lib/sim-manage.py", line 397, in main
    CommandDispatch()
  File "/home/jenkins/workspace/EinsteinToolkit/simfactory/lib/sim-manage.py", line 376, in CommandDispatch
    exec("command_%s()" % command)
  File "<string>", line 1, in <module>
  File "/home/jenkins/workspace/EinsteinToolkit/simfactory/lib/sim-manage.py", line 183, in command_create_run
    command_run()
  File "/home/jenkins/workspace/EinsteinToolkit/simfactory/lib/sim-manage.py", line 220, in command_run
    restart.userRun(simulationName)
  File "/home/jenkins/workspace/EinsteinToolkit/simfactory/lib/simrestart.py", line 956, in userRun
    self.copyTestsuiteData()
  File "/home/jenkins/workspace/EinsteinToolkit/simfactory/lib/simrestart.py", line 494, in copyTestsuiteData
    rsyncversion = simlib.RsyncVersion(rsyncInfo)
  File "/home/jenkins/workspace/EinsteinToolkit/simfactory/lib/simlib.py", line 1055, in RsyncVersion
    versionText = ExecuteCommand('%s --version' % rsynccmd, True)
  File "/home/jenkins/workspace/EinsteinToolkit/simfactory/lib/simlib.py", line 545, in ExecuteCommand
    if (not output) or (global_dict['VERBOSE']):
KeyError: 'VERBOSE'

The script which is running when this failure occurs is at https://bitbucket.org/ianhinder/cactusjenkins/src/master/test-cactus?at=master and the relevant command is presumably the

simfactory/bin/sim create-run --testsuite $tests --procs $cores --num-threads $((cores/nprocs)) $simname

Maybe you can reproduce the failure with a similar command line?

Changed 4 years ago by Steven R. Brandt

Attachment: sim3.patch added

Fixed the problem with accessing globals that was causing the test bot to fail.

comment:18 Changed 4 years ago by Roland Haas

Status: reopenedreview

comment:19 Changed 4 years ago by Roland Haas

Status: reviewreviewed_ok

Please apply (personal communication: Steve could reproduce the issue and fix it with the new patch).

comment:20 Changed 4 years ago by Steven R. Brandt

Resolution: fixed
Status: reviewed_okclosed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Steven R. Brandt.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.