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

Suppress heavy logs on test (fix #284) #285

Merged
merged 8 commits into from
May 10, 2022

Conversation

Hi-king
Copy link
Member

@Hi-king Hi-king commented May 8, 2022

Fixes #284

FIXES

    1. reconfigure luigi.setup_logging configs on each setUp and tearDown
    1. gokart.build() propagate log_level also to luigi namespace

PROBLEMS: heavy logs

https://github.com/m3dev/gokart/runs/6319432334?check_suite_focus=true

image

Reason1: cache of logging cofiguration

Reason2: gokart.build(log_level=...) only set log_level of gokart namespace

Info level logs from luigi namespace had been shown.

@@ -88,7 +92,7 @@ def test_build_dict_outputs(self):

def test_failed_task(self):
with self.assertRaises(GokartBuildError):
gokart.build(_DummyFailedTask(), reset_register=False)
gokart.build(_DummyFailedTask(), reset_register=False, log_level=logging.CRITICAL)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fail message is ERROR level, but not essential in this test

@@ -53,6 +53,8 @@ def run(self):
class RunTest(unittest.TestCase):

def setUp(self):
luigi.setup_logging.DaemonLogging._configured = False
luigi.setup_logging.InterfaceLogging._configured = False
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -31,7 +37,7 @@ def test_make_tree_info_complete(self):
task = _Task(param=1, sub=_SubTask(param=2))

# check after sub task runs
luigi.build([task], local_scheduler=True)
gokart.build(task, reset_register=False)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not essential change, but more handy to use gokart.build

Copy link
Collaborator

@hirosassa hirosassa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Copy link
Contributor

@mski-iksm mski-iksm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Hi-king LGTM!

@mski-iksm mski-iksm merged commit f391cff into m3dev:master May 10, 2022
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.

Heavy logs on test
3 participants