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

Make logging better for pycbc_make_offline_workflow #4517

Merged
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions bin/workflows/pycbc_make_offline_search_workflow
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,18 @@ def ifo_combos(ifos):
for ifocomb in combinations:
yield ifocomb


# Log to the screen until we know where the output file is
logging.basicConfig(format='%(asctime)s:%(levelname)s : %(message)s',
level=logging.INFO)

Copy link
Contributor

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?

Copy link
Contributor Author

@GarethCabournDavies GarethCabournDavies Oct 9, 2023

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

Copy link
Contributor Author

@GarethCabournDavies GarethCabournDavies Oct 9, 2023

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

Copy link
Contributor Author

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

parser = argparse.ArgumentParser(description=__doc__[1:])
parser.add_argument('--version', action='version', version=__version__)
parser.add_argument('--verbose', action='count',
help="Incrementally add more verbosity")
wf.add_workflow_command_line_group(parser)
wf.add_workflow_settings_cli(parser)
args = parser.parse_args()

# By default, we do logging.info, each --verbose adds a level of verbosity
logging_level = args.verbose + 1 if args.verbose else logging.INFO
pycbc.init_logging(logging_level)

container = wf.Workflow(args, args.workflow_name)
workflow = wf.Workflow(args, args.workflow_name + '-main')
finalize_workflow = wf.Workflow(args, args.workflow_name + '-finalization')
Expand Down
4 changes: 4 additions & 0 deletions pycbc/workflow/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"""

import os
import logging
import stat
import shutil
import subprocess
Expand Down Expand Up @@ -87,6 +88,9 @@ def resolve_url(url, directory=None, permissions=None, copy_to_cwd=True):
# it needs to be available when documentation runs in the CI, and I
# can't get it to install in the GitHub CI
import ciecplib
# Make the scitokens logger a little quieter
# (it is called through ciecpclib)
logging.getLogger('scitokens').setLevel(logging.root.level + 10)
with ciecplib.Session() as s:
if u.netloc in ("git.ligo.org", "code.pycbc.phy.syr.edu"):
# authenticate with git.ligo.org using callback
Expand Down
8 changes: 6 additions & 2 deletions pycbc/workflow/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,12 @@ def __init__(self, cp, name, ifos=None, out_dir=None, tags=None,
# being directly accessible on the execution site.
# CVMFS is perfect for this! As is singularity.
self.exe_pfns[exe_site] = exe_path
logging.info("Using %s executable "
"at %s on site %s" % (name, exe_url.path, exe_site))
logging.debug(
"Using %s executable at %s on site %s",
name,
exe_url.path,
exe_site
)

# FIXME: This hasn't yet been ported to pegasus5 and won't work.
# Pegasus describes two ways to work with containers, and I need
Expand Down
18 changes: 13 additions & 5 deletions pycbc/workflow/pegasus_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"""
import os
import shutil
import logging
import tempfile
import subprocess
from packaging import version
Expand Down Expand Up @@ -315,6 +316,13 @@ 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

# Get the logger associated with the Pegasus workflow import
pegasus_logger = logging.getLogger('Pegasus')
pegasus_logger.setLevel(logging.root.level + 10)
self.name = name
self._rc = dax.ReplicaCatalog()
self._tc = dax.TransformationCatalog()
Expand All @@ -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)
Expand Down Expand Up @@ -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.")
DeprecationWarning("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.")
Copy link
Contributor

Choose a reason for hiding this comment

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

This will not do anything actually; if you want the warning to be printed, you need something like

import warnings

warnings.warn('Blah', DeprecationWarning)

urlparts = urlsplit(path)
site = 'nonlocal'
if (urlparts.scheme == '' or urlparts.scheme == 'file'):
Expand Down
Loading