Colored output (issues #65) #141

lawrence910426 · 2023-03-20T06:00:55Z

Implemented issues #65.

This reverts commit cf7209b.

mcopik · 2023-03-22T15:25:42Z

sebs/color.py

+class ColoredPrinter:
+    @staticmethod
+    def print(color, logging_instance, message):
+        timestamp = datetime.datetime.now().strftime("%H:%M:%S")


Could we still display milliseconds in the timestamp?

Got it. Fixed :D

mcopik · 2023-03-22T16:56:19Z

sebs.py

-    output_dir = sebs.utils.create_output(output_dir, preserve_out, verbose)
-    sebs_client.logging.info("Created experiment output at {}".format(output_dir))
+    output_dir = sebs.utils.create_output(output_dir, preserve_out, verbose)    
+    ColoredPrinter.print(Colors.STATUS, sebs_client.logging, "Created experiment output at {}".format(output_dir))


This will work for all calls you replaced in the CLI file sebs.py. All other logging functionalities will not get colors.

We already have a logging base client. All other classes inherit from this class and later call self.logging.{logging_call}. Perhaps
Perhaps we could implement it there, such that every call in every class in the project would go through your coloring scheme?

One possible implementation is to add to the logging base a @property function called logging that would return your colored wrapper instead of the original logger. This should seamlessly affect the entire project.

I have implemented all wrappers for logging, critical, error, warning, info and debug. Each of them are mapped to the corresponding appropriate colors.

mcopik · 2023-03-22T16:56:39Z

sebs.py

@@ -121,8 +122,8 @@ def parse_common_params(
    logging_filename = os.path.abspath(os.path.join(output_dir, output_file))

    sebs_client = sebs.SeBS(cache, output_dir, verbose, logging_filename)
-    output_dir = sebs.utils.create_output(output_dir, preserve_out, verbose)
-    sebs_client.logging.info("Created experiment output at {}".format(output_dir))
+    output_dir = sebs.utils.create_output(output_dir, preserve_out, verbose)    


Please remove the trailing whitespace :-) Running tools/linting.py sebs should help.

I have reverted sebs.py so there is no need to worry about the trailing whitespaces :D

mcopik · 2023-03-22T16:57:20Z

sebs/color.py

@@ -0,0 +1,17 @@
+import click


I like the approach to a dedicated class! What do you think about placing it in sebs/utils.py instead of a separate file? It's quite small.

OK :D
I have put it in the very bottom of sebs/utils.py

mcopik · 2023-03-22T17:04:34Z

@lawrence910426 Thank you so much for your contribution! The CLI output looks much better already :-)

I left a few comments on the review - most of them are super simple. I think that if we move the class to LogginBase, this will be a really nice improvement since the entire project will be able to benefit from this improved output.

lawrence910426 · 2023-03-23T02:57:14Z

I have added a few more commits and fixed the problems mentioned. Thank you @mcopik again for your advising (^ ^/)

William-Mou · 2023-03-24T08:51:15Z

sebs.py

@@ -17,6 +17,7 @@
 from sebs.utils import update_nested_dict, catch_interrupt
 from sebs.faas import System as FaaSSystem
 from sebs.faas.function import Trigger
+from sebs.color import Colors, ColoredPrinter


@lawrence910426 This line may be unnecessary and may be forgotten to delete.

Good catch - this is now causing import error :)

mcopik · 2023-03-25T00:09:28Z

sebs.py

@@ -17,6 +17,7 @@
 from sebs.utils import update_nested_dict, catch_interrupt
 from sebs.faas import System as FaaSSystem
 from sebs.faas.function import Trigger
+from sebs.color import Colors, ColoredPrinter


Good catch - this is now causing import error :)

mcopik · 2023-03-25T00:47:58Z

sebs/utils.py

@@ -168,21 +171,26 @@ class LoggingBase:
    def __init__(self):
        uuid_name = str(uuid.uuid4())[0:4]
        if hasattr(self, "typename"):
-            self.logging = logging.getLogger(f"{self.typename()}-{uuid_name}")
+            self._logging = logging.getLogger(f"{self.typename()}-{uuid_name}")


We should also use the typename-uuid combination in the colored printer. It makes it much easier to debug experiments, especially the parallel ones.

Old

01:11:22,963 INFO AWS.HTTPTrigger-6c46: Invoke of function was successful

New, colored

[01:11:22.963650] Invoke of function was successful

The INFO part we do not need :-)

Okay, I got it fixed. Now it would output something like this.

mcopik · 2023-03-25T00:48:25Z

sebs/utils.py


    @property
    def logging_handlers(self) -> LoggingHandlers:
        return self._logging_handlers
+
+    @property
+    def logging(self) -> logging.Logger:


The static return type is not correct.

Got it. I have it fixed.

sebs/utils.py

mcopik · 2023-03-25T00:57:54Z

sebs/utils.py

+
+    def _print(self, message, color):
+        timestamp = datetime.datetime.now().strftime("%H:%M:%S.%f")
+        self.logging_instance.info(message)


I think that is the last remaining issue - right now, the output is duplicated because we have defined StreamHandler above. I think your approach of printing to logging_instance is correct because we want to also write to file. But we should now disable StreamHandler and configure the ColorPrinter with verbose flag to decide if debug should be printed or not,

I have amended ColoredPrinter to ColoredWrapper, which would only propagate logging instructions to _logger if filename in LoggingHandler is present. Moreover, StreamHandler is removed, and its functions are implemented in ColoredWrapper now.

mcopik · 2023-03-25T00:59:09Z

@lawrence910426 Thanks for your hard work! I think it's almost there :) Please make sure to test to verify that the output is correct, such that we do not get duplicated output :)

Looks really nice on my CLI!

…-benchmarks

lawrence910426 · 2023-03-25T18:13:36Z

Hello @mcopik, I have these issues fixed :)

Output typename-uuid for easier debugs.
The static return type is now ColoredWrapper which is a wrapper that imitate a normal logging instance.
Changed the order of properties for mypy to work
Duplicated output on stdout

I have verified the output and it fits our expectations.

Thank you again for your reviewing! This would surely made CLI more beautiful :)

mcopik · 2023-03-29T07:20:31Z

@lawrence910426 LGTM, thank you so much! This will make it much easier to catch warnings and errors, particularly in the verbose output mode :)

lawrence910426 added 5 commits March 20, 2023 11:44

feat: implement color wrapper

c6c700e

feat: replace logging with colored printer

29d2cd5

fix: output to logging

1195fd3

fix: stop output to stdout

cf7209b

Revert "fix: stop output to stdout"

49f0153

This reverts commit cf7209b.

mcopik requested changes Mar 22, 2023

View reviewed changes

lawrence910426 added 5 commits March 23, 2023 10:21

fix: revert intercepting on sebs.py

fc41871

feat: move color to utils.py

c9751e4

feat: miliseconds

158b697

feat: intercept @ logging base

0242cef

fix: rename vars to avoid conflict

41763f6

William-Mou reviewed Mar 24, 2023

View reviewed changes

mcopik requested changes Mar 25, 2023

View reviewed changes

lawrence910426 added 12 commits March 25, 2023 20:09

fix: remove legacy output

b90d725

fix: legacy imports

39758cd

fix: wrong return type

885b645

fix: mypy precedence

72bd7f3

fix: mypy precedence

613a12d

fix: change precedence of colored printer

603d0d4

fix: define return signature

5cb813c

feat: colored wrapper

16c8870

Merge branch 'master' of https://github.com/lawrence910426/serverless…

2550432

…-benchmarks

fix: update to single handler

e724d08

fix: propagate only if file input exists

1b7e399

feat: metadata before main message

b3ada3c

mcopik approved these changes Mar 29, 2023

View reviewed changes

mcopik merged commit 518fbbf into spcl:master Mar 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Colored output (issues #65) #141

Colored output (issues #65) #141

lawrence910426 commented Mar 20, 2023

mcopik Mar 22, 2023

lawrence910426 Mar 23, 2023

mcopik Mar 22, 2023

mcopik Mar 22, 2023

lawrence910426 Mar 23, 2023

mcopik Mar 22, 2023

lawrence910426 Mar 23, 2023

mcopik Mar 22, 2023

lawrence910426 Mar 23, 2023

mcopik commented Mar 22, 2023

lawrence910426 commented Mar 23, 2023

William-Mou Mar 24, 2023

mcopik Mar 25, 2023

mcopik Mar 25, 2023

mcopik Mar 25, 2023

lawrence910426 Mar 25, 2023

mcopik Mar 25, 2023

lawrence910426 Mar 25, 2023

mcopik Mar 25, 2023

lawrence910426 Mar 25, 2023

mcopik commented Mar 25, 2023

lawrence910426 commented Mar 25, 2023

mcopik commented Mar 29, 2023

Colored output (issues #65) #141

Colored output (issues #65) #141

Conversation

lawrence910426 commented Mar 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcopik commented Mar 22, 2023

lawrence910426 commented Mar 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcopik commented Mar 25, 2023

lawrence910426 commented Mar 25, 2023

mcopik commented Mar 29, 2023