Skip to content

Ensure docs are complete and correct with respect to SEVERE vs ERROR#11

Merged
infotroph merged 7 commits intoinfotroph:logger-oldvalsfrom
dlebauer:logger_docs
Aug 6, 2025
Merged

Ensure docs are complete and correct with respect to SEVERE vs ERROR#11
infotroph merged 7 commits intoinfotroph:logger-oldvalsfrom
dlebauer:logger_docs

Conversation

@dlebauer
Copy link

@dlebauer dlebauer commented Aug 6, 2025

Written while reviewing of PecanProject#3589

@infotroph please advise if you prefer I target this PR to pecan/develop instead of your PR.

Summary

  • Updates documentation to make sure SEVERE log level is consistently documented
  • Fixes a bug in logger.getLevelNumber where ERROR and SEVERE were assigned the same value. I believe that this is why logger.severe does not stop workflows like it should.
  • Clarifies that logger.setLevel should be used instead of logger.getLevel for temporary changes (since I realized during review of 3589 that setLevel returns the old level)

Question for discussion

What is the desired behavior for SEVERE, and what is the desired default?

My opinion: by default, logger.severe should stop execution and throw an error that can be caught in testing with testthat::expect_error.

Currently, the logger.setQuitOnSevere docs are unclear:

First, the default set at top of logger.R

.utils.logger$quit <- FALSE

But the logger.setQuitOnSevere docs indicate that default is TRUE, although I don't fully understand the desired behavior when run interactively, inside Rstudio, or when scripted. And what makes Rstudio special? Is that just a shorthand for 'interactive' - would the same thing happen when run in Rstudio as a background job?

##' set \code{\link{logger.setQuitOnSevere}(FALSE)} to avoid terminating
##' the session. This is set by default to TRUE if interactive or running
##' inside Rstudio.

@dlebauer dlebauer changed the title Ensure docs are complete and correct(?) with respect to SEVERE vs ERROR Ensure docs are complete and correct with respect to SEVERE vs ERROR Aug 6, 2025
return(40)
} else if (toupper(level) == "SEVERE") {
return(40)
return(50)
Copy link
Owner

Choose a reason for hiding this comment

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

👍

dlebauer and others added 5 commits August 6, 2025 15:24
Co-authored-by: Chris Black <chris@ckblack.org>
Co-authored-by: Chris Black <chris@ckblack.org>
Co-authored-by: Chris Black <chris@ckblack.org>
Co-authored-by: Chris Black <chris@ckblack.org>
Co-authored-by: Chris Black <chris@ckblack.org>
@infotroph infotroph merged commit 43295b2 into infotroph:logger-oldvals Aug 6, 2025
5 of 21 checks passed
@dlebauer dlebauer deleted the logger_docs branch August 6, 2025 22:53
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.

2 participants