-
Notifications
You must be signed in to change notification settings - Fork 28.1k
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
Enable pytest live log and show warning logs on GitHub Actions CI runs #35912
Conversation
run-slow: convnextv2 |
1 similar comment
run-slow: convnextv2 |
This comment contains run-slow, running the specified jobs: ['models/convnextv2'] ... |
run-slow: convnextv2 |
This comment contains run-slow, running the specified jobs: ['models/convnextv2'] ... |
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
See a run with the live log (for warnings) https://github.com/huggingface/transformers/actions/runs/13008702695/job/36281380509 |
@@ -101,7 +101,8 @@ def _configure_library_root_logger() -> None: | |||
formatter = logging.Formatter("[%(levelname)s|%(pathname)s:%(lineno)s] %(asctime)s >> %(message)s") | |||
_default_handler.setFormatter(formatter) | |||
|
|||
library_root_logger.propagate = False | |||
is_ci = os.getenv("CI") is not None and os.getenv("CI").upper() in {"1", "ON", "YES", "TRUE"} | |||
library_root_logger.propagate = True if is_ci else False |
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.
We need propagate = True
in order to get live log (here for warnings
), but let's only do it when CI
env. variable is set.
This variable is set on GitHub Actions and CircleCI.
On CircleCI, we use -n xxxx
(xdist
) which won't show live log, which is preferred (to keep it more quite).
On GitHub Actions, we don't use -n xxx
and live log is shown, so we can grab logs with warning info.
@@ -785,8 +785,7 @@ def validate(self, is_init=False): | |||
for arg_name in ("cache_implementation", "cache_config", "return_legacy_cache"): | |||
if getattr(self, arg_name) is not None: | |||
logger.warning_once( | |||
no_cache_warning.format(cache_arg=arg_name, cache_arg_value=getattr(self, arg_name)), | |||
UserWarning, |
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.
Having UserWarning
(or anything else as 2nd arg.) is not going to work. It gives
def getMessage(self):
"""
Return the message for this LogRecord.
Return the message for this LogRecord after merging any user-supplied
arguments with the message.
"""
msg = str(self.msg)
if self.args:
> msg = msg % self.args
E TypeError: not all arguments converted during string formatting
Previously, some log handler can handle the above exception:
- <StreamHandler <tempfile._TemporaryFileWrapper object at 0x000002AC2614E850> (NOTSET)>
- <_LiveLoggingStreamHandler (WARNING)>,
- <_FileHandler \.\nul (NOTSET)>
But after this PR, a new extra handle is used and the exception is kept and thrown
- <LogCaptureHandler (NOTSET)>,
run-slow: convnextv2 |
This comment contains run-slow, running the specified jobs: ['models/convnextv2'] ... |
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.
Completely missed the ping sorry!
run-slow: convnextv2 |
This comment contains run-slow, running the specified jobs: This comment contains run-slow, running the specified jobs: models: ['models/convnextv2'] |
root = _get_library_root_logger() | ||
with patch.object(root, "propagate", False): |
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.
It turns out that live log will interfere with CaptureStdout
and cause this test failed.
Simply patch the root logger's propagate
within with
here.
run-slow: convnextv2 |
This comment contains run-slow, running the specified jobs: This comment contains run-slow, running the specified jobs: models: ['models/convnextv2'] |
huggingface#35912) * fix * remove * fix --------- Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>
We want to have warnings being logged on GitHub Actions run.
(currently on
main
, only those given bywarnings.warn(...)
are saved when using--make-reports
)