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
DM-41229: Distribute log files across subdirs. #22
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #22 +/- ##
==========================================
+ Coverage 39.28% 47.32% +8.04%
==========================================
Files 10 11 +1
Lines 336 374 +38
Branches 60 61 +1
==========================================
+ Hits 132 177 +45
+ Misses 198 191 -7
Partials 6 6
☔ View full report in Codecov by Sentry. |
"replaceVars": False, | ||
"default": "", | ||
}, | ||
) |
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.
get_bps_config_value
exists to simplify these kind of lookups. What are you trying to do that it does not accomplish?
Is a default of ""
suitable?
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.
Am I missing something in the get_bps_config_value
code? I need to be able to not replace the variables in the value returned (
options: dict[str, Any] = dict(expandEnvVars=True, replaceVars=True, required=required) |
GenericWorkflowJob
and I need to pass in other search options (the curvals) but don't see a way to do that in get_bps_config_value
.
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.
OK, that sounds rather more complicated than get_bps_config_value
can handle. We could extend it, but maybe it's not worth it for a single use case.
a22b6a8
to
b0fb04b
Compare
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.
Great, thanks! I'm especially grateful for the test.
doc/lsst.ctrl.bps.parsl/use.rst
Outdated
@@ -52,6 +52,9 @@ The following configuration settings can be used in configuring the plugin: | |||
* ``parsl.log_level`` (`str`): logging level for Parsl; may be one of ``CRITICAL``, ``DEBUG``, ``ERROR``, ``FATAL``, ``INFO``, ``WARN``. | |||
* ``project`` (`str`): project name; defaults to ``bps``. | |||
* ``campaign`` (`str`): campaign name; defaults to the user name (which can also be set via the ``username`` setting). | |||
* ``subDirTemplate`` (`str`): template used to define log subdirectories in order to avoid having too many files in a single directory; defaults to a very generic template defined by ctrl_bps in bps_defaults.yaml_. To run with no subdirectories (original plugin behavior), in the submit yaml set ``subDirTemplate`` to the empty string (``subDirTemplate: ''``). |
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 don't think there's a need to mention "original plugin behavior".
b0fb04b
to
32fb5b5
Compare
Checklist
doc/changes