-
Notifications
You must be signed in to change notification settings - Fork 80
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
Assert messages to flush to the log file #923
Assert messages to flush to the log file #923
Conversation
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
Is this good to go @benmalef? |
@sarthakpati |
@VukW do you have any feedback? |
Hey folks, from a first glance it seems to be a legit way to log an exception, but to tell truth I'm confused (I've never used |
Any update, @VukW? |
@benmalef does PR work for you? For me |
@VukW How do you test it? |
Important NotesI have tried using TestingI tried testing it, but pytest has its own implementation for handling AssertionError. One possible solution could be this approach, which I attempted, but I couldn’t figure it out. Another method I tried was to modify the behavior of the assert statement in pytest. I followed some steps mentioned in this reference, but I ended up with the same result. To disable the default behavior, you can refer to this guide. |
Hey folks! Just to be clear, I've tested config manually with a following setup. logging.debug("this is logging.DEBUG") # <- debug log
logging.info("this is logging.INFO") # <- debug log and stdout
logging.warning("this is logging.WARNING") # <- debug log and stderr
logging.error("this is logging.ERROR") # <- debug log and stderr
logging.exception("this is logging.EXCEPTION") # <- debug log and stderr
warnings.warn("a WARNING!!") # debug log and stderr
logger = logging.getLogger("my-logger")
logger.debug("this is logger.DEBUG") # <- debug log
logger.info("this is logger.INFO") # <- debug log and stdout
logger.warning("this is logger.WARNING") # <- debug log and stderr
logger.error("this is logger.ERROR") # <- debug log and stderr
logger.exception("this is logger.EXCEPTION") # <- debug log and stderr
raise AssertionError("test failure") # <- debug log and stderr Here for every record I've listed where it should be sent. gandlf anonymizer -i . -o . --log-file gandlf.log 1>out.log 2>err.log Thus, I 'm able to see exactly which lines go to which log destination.
|
I think with this, we can proceed with the merge. I'll wait for the tests to finish. @VukW unless you have any feedback, can I proceed with resolving your comments? |
@sarthakpati sure, i don't have any other comments except already mentioned ones |
Co-authored-by: Viacheslav Kukushkin <[email protected]>
Fixes #913
Proposed Changes
Checklist
CONTRIBUTING
guide has been followed.typing
is used to provide type hints, including and not limited to usingOptional
if a variable has a pre-defined value).pip install
step is needed for PR to be functional), please ensure it is reflected in all the files that control the CI, namely: python-test.yml, and all docker files [1,2,3].