-
Notifications
You must be signed in to change notification settings - Fork 356
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
GarethCabournDavies
merged 8 commits into
gwastro:master
from
GarethCabournDavies:pegasus_verbosity
Oct 10, 2023
Merged
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
29ed427
Make logging better for pycbc_make_offline_workflow
GarethCabournDavies 42b5e5d
comment
GarethCabournDavies 762e547
Fix bug
GarethCabournDavies da16019
CC complaint - there are some other, but these havent been changed by…
GarethCabournDavies e3c5595
CC
GarethCabournDavies e3e349b
Update pycbc/workflow/core.py
GarethCabournDavies 05da66b
IH/TDC comments
GarethCabournDavies 8ba4d80
use warnings module
GarethCabournDavies File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 | ||
|
@@ -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() | ||
|
@@ -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.") | ||
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.") | ||
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. 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'): | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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