-
Notifications
You must be signed in to change notification settings - Fork 358
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
Make logging better for pycbc_make_offline_workflow #4517
Changes from 5 commits
29ed427
42b5e5d
762e547
da16019
e3c5595
e3e349b
05da66b
8ba4d80
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,7 @@ | |
""" | ||
import os | ||
import shutil | ||
import logging | ||
import tempfile | ||
import subprocess | ||
from packaging import version | ||
|
@@ -38,6 +39,9 @@ | |
PEGASUS_FILE_DIRECTORY = os.path.join(os.path.dirname(__file__), | ||
'pegasus_files') | ||
|
||
# Get the logger associated with the Pegasus workflow import | ||
pegasus_logger = logging.getLogger('Pegasus') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As you only access this in one place, it doesn't need to be a global variable, just do this above line 325 (unless I'm missing something??) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. moved - I had thought it would be needed elsewhere, but its just the once |
||
|
||
|
||
class ProfileShortcuts(object): | ||
""" Container of common methods for setting pegasus profile information | ||
|
@@ -315,6 +319,10 @@ class Workflow(object): | |
""" | ||
def __init__(self, name='my_workflow', directory=None, cache_file=None, | ||
dax_file_name=None): | ||
# Pegasus logging is fairly verbose, quieten it down a bit | ||
# This sets the logger to one level less verbose than the root | ||
# (pycbc) logger | ||
pegasus_logger.setLevel(logging.root.level + 10) | ||
self.name = name | ||
self._rc = dax.ReplicaCatalog() | ||
self._tc = dax.TransformationCatalog() | ||
|
@@ -340,7 +348,7 @@ def __init__(self, name='my_workflow', directory=None, cache_file=None, | |
self.filename = dax_file_name | ||
self._adag = dax.Workflow(self.filename) | ||
|
||
# A pegasus job version of this workflow for use if it isncluded | ||
# A pegasus job version of this workflow for use if it is included | ||
# within a larger workflow | ||
self._as_job = SubWorkflow(self.filename, is_planned=False, | ||
_id=self.name) | ||
|
@@ -843,10 +851,10 @@ def insert_into_dax(self, rep_cat, sites): | |
@classmethod | ||
def from_path(cls, path): | ||
"""Takes a path and returns a File object with the path as the PFN.""" | ||
logging.warn("The from_path method in pegasus_workflow is deprecated. " | ||
"Please use File.from_path (for output files) in core.py " | ||
"or resolve_url_to_file in core.py (for input files) " | ||
"instead.") | ||
logging.warning("The from_path method in pegasus_workflow is " | ||
"deprecated. Please use File.from_path (for output " | ||
"files) in core.py or resolve_url_to_file in core.py " | ||
"(for input files) instead.") | ||
titodalcanton marked this conversation as resolved.
Show resolved
Hide resolved
|
||
urlparts = urlsplit(path) | ||
site = 'nonlocal' | ||
if (urlparts.scheme == '' or urlparts.scheme == 'file'): | ||
|
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 move change anything?
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.
Don't think it does - looks like the workflow log file is created correctly, and added to the results page as normal
The only difference I see is that we can now configure to use more verbosity by passing the option
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.
Ahh - Ive just seen that there is the second call to the basicConfig to send to the log file
The change to init_logging also makes the default format change to use ISO standard time format including timezone and removes the level
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 suppose the question is which format we want, using the logging level, or the updated timezone format (or neither or both)
I don't want to revert to the direct basicConfig call, as then we would be unable to run with logging.DEBUG and would just lose all of the information we are wanting to make quieter