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 provenance information in log optional #2328

Merged
merged 6 commits into from Jun 30, 2023
Merged

Make provenance information in log optional #2328

merged 6 commits into from Jun 30, 2023

Conversation

Tobychev
Copy link
Contributor

The inclusion of all the packages in the environment into the provenance information makes it so long it usually completely dominates the line count of any logfile. This change adds a option that defaults to not including provenance in the log (in addition to it being written to a file).

@kosack
Copy link
Contributor

kosack commented May 11, 2023

Another option here is simply to never write the provenance to the stdout logfile. I only had added that for debugging before I had added the provenance.log output. But now that there is a provenance log, it's probably redundant to include it at the DEBUG level.

That way, no new option is needed. I don't really see a use case for turning it on again.

What might be useful in the DEBUG output is just to print the configuration (either from tool.get_current_config() or from the provenance info), but not the complete provenance dictionary

@@ -173,6 +173,13 @@ def main():
help="Logging Level for File Logging",
).tag(config=True)

log_include_provenance = Bool(
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't add an option for this (it's somewhat confusing to have control options affect only debugging output). Better to just decrease the log level of that print out (could set it to 5, which is below debug), or as I mentioned below, simply remove the printout altogether. I think I prefer the latter option to just drop it, as we already print it in the provenance file.

Copy link
Member

Choose a reason for hiding this comment

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

We could also add use a specific provenance sublogger so it's configurable on its own.

self.log.getChild("provenance").debug(...)

@Tobychev
Copy link
Contributor Author

@kosack @maxnoe I decided for the "remove it" option, with a note in the logs where for where the interested reader might find the provenance.

@kosack kosack merged commit 0f97f75 into main Jun 30, 2023
13 checks passed
@maxnoe maxnoe deleted the pipe-down branch June 30, 2023 12:10
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.

None yet

3 participants