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

Enable pytest live log and show warning logs on GitHub Actions CI runs #35912

Merged
merged 5 commits into from
Feb 10, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,5 @@ markers = [
"bitsandbytes: select (or deselect with `not`) bitsandbytes integration tests",
"generate: marks tests that use the GenerationTesterMixin"
]
log_cli = 1
log_cli_level = "WARNING"
3 changes: 1 addition & 2 deletions src/transformers/generation/configuration_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Collaborator Author

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)>,

no_cache_warning.format(cache_arg=arg_name, cache_arg_value=getattr(self, arg_name))
)

# 6. check watermarking arguments
Expand Down
3 changes: 2 additions & 1 deletion src/transformers/utils/logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator Author

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.



def _reset_library_root_logger() -> None:
Expand Down