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

None of the --log-level cli options work #121

Open
sbquinlan opened this issue Nov 1, 2023 · 6 comments
Open

None of the --log-level cli options work #121

sbquinlan opened this issue Nov 1, 2023 · 6 comments

Comments

@sbquinlan
Copy link
Contributor

What I did:

I wanted to change the default log level. I saw some properties on BaseCommand in the source so I looked at the help to see how to set them:

% pangeo-forge-runner --help                                                                             
# ... blah blah blah ...
--log-level=<Enum>
    Set the log level by value or name.
    Choices: any of [0, 10, 20, 30, 40, 50, 'DEBUG', 'INFO', 'WARN', 'ERROR', 'CRITICAL']
    Default: 30
    Equivalent to: [--Application.log_level]

Cool I'll set that:

% pangeo-forge-runner bake --log-level=CRITICAL

What happened:

[Bake] WARNING | Config option `log_level` not recognized by `Bake`.
[Bake] WARNING | Config option `log_level` not recognized by `Bake`.
# ... followed by info logs

I tried --log-level=0, --Application.log_level=WARN and got more or less the same result

What I expected

Bake accepts the log level config and does not INFO log.

@ranchodeluxe
Copy link
Collaborator

This this is a custom job here: https://github.com/pangeo-forge/pangeo-forge-runner/blob/main/pangeo_forge_runner/commands/base.py#L44C6-L57

Will look into later after some meetings

@ranchodeluxe
Copy link
Collaborator

ranchodeluxe commented Nov 1, 2023

welp, this didn't mute it either --Bake.logging_config='{"version": 1, "loggers": {"pangeo_forge_recipes":{"level": "CRITICAL", "handlers":[], "propagate": False}}}'

@ranchodeluxe
Copy link
Collaborator

Even this didn't work 😠

@dataclass
class ShutUpAndStoreToZarr(StoreToZarr):
    def expand(self, pcoll: beam.PCollection) -> beam.PCollection:
        rechunking_logger = logging.getLogger('pangeo_forge_recipes.rechunking')
        rechunking_logger.setLevel(logging.CRITICAL)
        return pcoll | StoreToZarr(store_name=self.store_name, combine_dims=self.combine_dims, target_chunks=self.target_chunks)

@yuvipanda
Copy link
Collaborator

The default logging config is for traitlets, and anything using self.log from objects that inherit from LoggingConfigurable.

I remember fiddling with it in #65, which may be of use :)

@ranchodeluxe
Copy link
Collaborator

ranchodeluxe commented Nov 3, 2023

The default logging config is for traitlets, and anything using self.log from objects that inherit from LoggingConfigurable.

I remember fiddling with it in #65, which may be of use :)

Right, what we're really after in this issue (which isn't explicit) is how to turn off logs in the default logging in recipe transforms b/c @sbquinlan was thinking they are causing issues with some grpc calls (at least I think that's what he was saying in DMs)

@sbquinlan
Copy link
Contributor Author

what we're really after in this issue (which isn't explicit)

Actually I think the explicit problem that I described at the start is good for this issue. The traitlet option for setting the log level does not seem to work at all. Is --log-level supposed to work?

This also changes the default local one from multi_processing to multi_threading. The primary use here is to make the logging config from the runner be inherited by the workers too, which helps with debugging quite a bit.

Is this saying that, when running locally, Bake.log_level should propagate to the workers? That's cool, but I'm not sure I saw that happening. It's unclear from your PR how that was tested for me to have a workaround here.

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

No branches or pull requests

3 participants