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

First draft of more fine-grained logging #198

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

thorehusfeldt
Copy link
Contributor

@thorehusfeldt thorehusfeldt commented Jun 8, 2021

Moves most of the functionality from ProblemAspect
into various filters, loggers, and handlers in logger.py.

Why is this good?

  • closer to how python's own logging facility handles logging (https://docs.python.org/3/library/logging.html).
  • allows more flexible and fine-grained logging, in particular
    • easy to add a file handler that sends DEBUG or INFO log to /tmp
    • attractive and clean way to add alternative formatting or more fine-grained logs (say, judgemessages or graders). This is the original motivation behind this refactoring.
  • decreases dependencies in verifyproblem's class structure (ideally, would get rid of ProblemAspect) by decoupling logging (log.error) from problem parts/aspects (self.get_score_range())

Why is this bad?

  • considerable refactoring of code

Stuff to think about before moving on:

  • Is this worth doing at all?
  • Can the attribute ProblemAspect._check_res be moved to InputValidators? Seems to be used only there.
  • How fine-grained should the logging hierarchy be? Currently the leaves follow the same granularity as does ProblemAspect (for instance, verifyproblem.hello.submissions), but one could go all the way down to test case or submission level verifyproblem.hello.test case group.group 3/045-huge.in or verifyproblem.hello.submissions.ac.th.py.

Moves most of the functionality from ProblemAspect
into various filters, loggers, and handlers in logger.py.
@pehrsoderman
Copy link
Contributor

@thorehusfeldt This needs a rebase.

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

Successfully merging this pull request may close these issues.

2 participants