Skip to content
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 INFO log about completed external commands #81

Merged
merged 10 commits into from
Mar 9, 2016

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented Mar 7, 2016

This should allow timestamps for long-running commands like DB upgrade to be printed/saved before and after the operation for easier measurements. See ome/omero-documentation#1424 (comment) for instance.

This should allow timestamps for long-running commands like DB upgrade to be
printed/saved before and after the operation for easier measurements
@joshmoore
Copy link
Member

Calculate the ms and include in both the INFO as well as the exception in case there was an error?

@manics
Copy link
Member

manics commented Mar 7, 2016

If you're keeping it at INFO could you put a more consise message, e.g. Just Command completed since the context should be clear? Or change to DEBUG?

@sbesson
Copy link
Member Author

sbesson commented Mar 7, 2016

Pushed

if r != 0:
raise RunException(
"Non-zero return code", exe, args, r, stdout, stderr)
"Non-zero return code [%s s]" % (end - start), exe, args, r,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems a bit strange to be putting timings in the exception message. How about improve the logging instead: log.error("Non-zero return code %d [%s s]", r, (end - start)) before the raise?

@manics
Copy link
Member

manics commented Mar 8, 2016

2016-03-08 11:25:01,905 [omego.extern] INFO  Executing [custom environment]: psql -d omero -h localhost -U omero -w -A -t --version
2016-03-08 11:25:01,928 [omego.extern] INFO  Completed [0.0222570896149 s]
2016-03-08 11:25:01,928 [    omego.db] INFO  psql version: psql (PostgreSQL) 9.4.6
2016-03-08 11:25:01,941 [omego.extern] INFO  Executing [custom environment]: pg_dump -d omero -h localhost -U omero -w -Fc -f omero-database-omero-20160308-112501-940790.pgdump
2016-03-08 11:25:01,945 [omego.extern] ERROR Failed [0.00365400314331 s]
Traceback (most recent call last):

Very minor nitpick: how about format it as %.3f? And then it's good to merge.

@sbesson
Copy link
Member Author

sbesson commented Mar 8, 2016

Done

raise RunException(
"Non-zero return code", exe, args, r, stdout, stderr)
log.info("Completed [%s s]", end - start)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed one :-( %.3f

@sbesson
Copy link
Member Author

sbesson commented Mar 9, 2016

Pushed again.

@manics
Copy link
Member

manics commented Mar 9, 2016

Thanks

manics added a commit that referenced this pull request Mar 9, 2016
Add INFO log about completed external commands
@manics manics merged commit 2ea006e into ome:master Mar 9, 2016
@sbesson sbesson deleted the external_completion_timestamp branch March 9, 2016 09:42
@sbesson sbesson added this to the 0.4.0 milestone Jun 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants