feat: added set_logging_level functionality#372
feat: added set_logging_level functionality#372pranayyb wants to merge 7 commits intoPrunaAI:mainfrom
Conversation
johannaSommer
left a comment
There was a problem hiding this comment.
Looks perfect to me, thanks a lot @pranayyb for taking the time to enable it! Just have a quick question about the environment variable, then we can already merge!
| } | ||
|
|
||
|
|
||
| def set_logging_level(level: str | None = None) -> None: |
There was a problem hiding this comment.
Say the user sets the environment variable before running the script - the way i understand the code currently they would still have to call set_logging_level, is that correct? Can we implement it so that the logger - when first set up - respects this environment variable? Do you think that makes sense?
There was a problem hiding this comment.
yes i see, i'll change accordingly
|
@johannaSommer could you please review now? |
johannaSommer
left a comment
There was a problem hiding this comment.
Thanks a lot for the change, I think now it looks great w.r.t. respecting the environment variable at setup! Is there a reason the user now has to pass the logger? In pruna, we currently only have one so it would be easier if the user only gives the logging level as an argument to set_logging_level? Otherwise LGTM, I will already run the test workflows so that we can merge after adressing that 😊
|
I did think of that without having to pass the logger but using set_logging_level required purna logger already defined and to define purna logger we need to call setup_peuna_logger which needed set_logging_level as you said at setup. Here the cursor bot raised that issue so I changed it that way. Please let me know if any changes required. |
|
hey @johannaSommer this was getting messy so raised a new clean PR with the final changes. |
Description
Added a utility function set_logging_level to configure the global pruna_logger logging level programmatically or via the PRUNA_LOG_LEVEL environment variable. Also added unit tests to verify correct behavior for valid and invalid logging levels.
Related Issue
Fixes #351
Type of Change
How Has This Been Tested?
DEBUG, INFO, WARNING, ERROR,CRITICAL)PRUNA_LOG_LEVEL)Checklist
Additional Notes
Note
Introduce set_logging_level to configure logger level via argument or PRUNA_LOG_LEVEL and update logger setup; add tests for valid/invalid levels.
set_logging_level(logger, level)with validation and_LOG_LEVELSmap; readsPRUNA_LOG_LEVELwhenlevelis None.setup_pruna_logger()to useset_logging_level(defaulting via env) and adjust docstring accordingly.tests/config/test_logging.pycovering programmatic level setting, env-driven setting, and invalid level error.Written by Cursor Bugbot for commit 5acaf26. This will update automatically on new commits. Configure here.