enable timerotateloghandler (configurable) by sid88in · Pull Request #311 · apache/superset

sid88in · 2016-04-10T03:12:59Z

No description provided.

coveralls · 2016-04-10T03:21:34Z

Coverage increased (+0.2%) to 72.536% when pulling ef97c65 on sid88in:enhance_logging into f8e5d30 on airbnb:master.

sid88in · 2016-04-10T03:32:57Z

@mistercrunch I am testing this PR to enhance logging in caravel. Reference:

mistercrunch · 2016-04-10T07:00:41Z

+2) https://docs.python.org/2/library/logging.config.html
+"""
+
+from caravel import app


I think that creates a circular import

mistercrunch · 2016-04-10T07:04:00Z

The build failed because of what I'm guessing is a circular import.

I don't think we need a specific module for just the logging config, I'd squeeze that into conf.py instead. To avoid circular imports you can simply use a function that receives the app configuration and returns the loggingconfig object perhaps.

sid88in · 2016-04-10T15:23:25Z

Cool. Looking into it.

coveralls · 2016-04-10T18:25:02Z

Coverage increased (+0.2%) to 72.525% when pulling 58825e6 on sid88in:enhance_logging into f8e5d30 on airbnb:master.

coveralls · 2016-04-10T21:16:30Z

Coverage increased (+0.2%) to 72.547% when pulling 9452905 on sid88in:enhance_logging into f8e5d30 on airbnb:master.

coleslaw · 2016-04-10T21:45:48Z

 from caravel import db

 config = app.config
+logging.config.dictConfig(config.get('LOGGING_CONFIG'))


is this line needed here and everywhere else?
It's already called in caravel/__init__.py and that is executed whenever any caravel module is imported.

@sid88in

Thats a good catch. I tested it after removing the redundant declaration and it works.

…avel/__init__.py

coveralls · 2016-04-10T22:40:49Z

Coverage increased (+0.1%) to 72.47% when pulling 50bb311 on sid88in:enhance_logging into f8e5d30 on airbnb:master.

mistercrunch · 2016-04-10T23:20:51Z

    """Starts a Caravel web server"""
-    debug = debug or config.get("DEBUG")
-    if debug:
+    debug = config.get('LOG_LEVEL')


debug meant something else here (whether the web server should run in debug mode). We should keep that insulated from the logging level.

I see. I noticed that but was wondering what if debug = True and Log Level = Debug. I have reverted back the setting for running web server as debug mode true/false

coveralls · 2016-04-11T00:04:28Z

Coverage increased (+0.4%) to 72.695% when pulling e408332 on sid88in:enhance_logging into f8e5d30 on airbnb:master.

coveralls · 2016-04-11T00:16:49Z

Coverage increased (+0.4%) to 72.695% when pulling e408332 on sid88in:enhance_logging into f8e5d30 on airbnb:master.

coveralls · 2016-04-11T00:26:04Z

Coverage increased (+0.4%) to 72.695% when pulling e408332 on sid88in:enhance_logging into f8e5d30 on airbnb:master.

landscape-bot · 2016-04-11T05:09:12Z

Repository health decreased by 0.11% when pulling e408332 on sid88in:enhance_logging into f8e5d30 on airbnb:master.

2 new problems were found (including 0 errors and 1 code smell).
No problems were fixed.

landscape-bot · 2016-04-11T05:14:12Z

Repository health decreased by 0.11% when pulling e408332 on sid88in:enhance_logging into f8e5d30 on airbnb:master.

2 new problems were found (including 0 errors and 1 code smell).
No problems were fixed.

landscape-bot · 2016-04-11T05:16:45Z

Repository health decreased by 0.11% when pulling e408332 on sid88in:enhance_logging into f8e5d30 on airbnb:master.

2 new problems were found (including 0 errors and 1 code smell).
No problems were fixed.

landscape-bot · 2016-04-11T05:21:57Z

Repository health decreased by 0.11% when pulling e408332 on sid88in:enhance_logging into f8e5d30 on airbnb:master.

2 new problems were found (including 0 errors and 1 code smell).
No problems were fixed.

coveralls · 2016-04-11T05:52:00Z

Coverage increased (+0.3%) to 72.684% when pulling 7f82849 on sid88in:enhance_logging into f8e5d30 on airbnb:master.

mistercrunch · 2016-04-11T22:49:35Z

Ok sorry about the incremental comments, I should have written all comments at once early on.

So I think the normal assumption is that a piece of software logs to STDOUT only so that if I run it with something like runit or sv it will do its own log rotation and all that. Can we make the default to only log to STDOUT and make it easy and clear for people to configure it to log differently?

coveralls · 2016-04-17T01:38:42Z

Coverage increased (+0.04%) to 72.511% when pulling e5ae4ab on sid88in:enhance_logging into 899fe19 on airbnb:master.

landscape-bot · 2016-04-17T01:40:10Z

Repository health decreased by 0.01% when pulling e624c41 on sid88in:enhance_logging into 899fe19 on airbnb:master.

1 new problem was found (including 0 errors and 0 code smells).
No problems were fixed.

sid88in · 2016-04-17T01:43:17Z

@mistercrunch I have configured timeRotateLogRotation which is often needed when this application runs over large scale - logs can then be distributed across multiple files automatically based on rollover time (in this case every midnight new file will be created). Also I have reverted back all logger statements to print (in master), so people can configure how they want to log. TimeRotate is also configurable in conifg.

landscape-bot · 2016-04-17T01:43:31Z

Code quality remained the same when pulling 1f81a98 on sid88in:enhance_logging into 899fe19 on airbnb:master.

landscape-bot · 2016-04-17T01:43:39Z

Code quality remained the same when pulling 1f81a98 on sid88in:enhance_logging into 899fe19 on airbnb:master.

coveralls · 2016-04-17T01:46:18Z

Coverage decreased (-0.03%) to 72.443% when pulling 8cd0764 on sid88in:enhance_logging into 899fe19 on airbnb:master.

landscape-bot · 2016-04-17T01:46:32Z

Code quality remained the same when pulling 8cd0764 on sid88in:enhance_logging into 899fe19 on airbnb:master.

coveralls · 2016-04-17T01:47:01Z

Coverage decreased (-0.03%) to 72.443% when pulling 8cd0764 on sid88in:enhance_logging into 899fe19 on airbnb:master.

coveralls · 2016-04-17T01:56:56Z

Coverage decreased (-0.002%) to 72.468% when pulling 8cd0764 on sid88in:enhance_logging into 899fe19 on airbnb:master.

coveralls · 2016-04-17T01:57:03Z

Coverage increased (+0.001%) to 72.471% when pulling 8cd0764 on sid88in:enhance_logging into 899fe19 on airbnb:master.

coveralls · 2016-04-17T02:00:05Z

Coverage increased (+0.001%) to 72.471% when pulling 8cd0764 on sid88in:enhance_logging into 899fe19 on airbnb:master.

mistercrunch · 2016-04-19T03:38:22Z


+# Logging configuration
+logging.basicConfig(format='%(asctime)s:%(levelname)s:%(name)s:%(message)s')
+logging.getLogger().setLevel(logging.DEBUG)


if only two things was to get parameterized it should be the logging level and the format string

landscape-bot · 2016-04-19T04:15:39Z

Code quality remained the same when pulling 956c84d on sid88in:enhance_logging into 899fe19 on airbnb:master.

mistercrunch · 2016-04-30T06:54:21Z

This is good to go, you just need to rebase...

# Conflicts: # caravel/config.py # caravel/viz.py

landscape-bot · 2016-05-01T00:23:31Z

Code quality remained the same when pulling 9f328e5 on sid88in:enhance_logging into 7b5b602 on airbnb:master.

coveralls · 2016-05-01T00:24:02Z

Coverage decreased (-0.02%) to 80.967% when pulling 9f328e5 on sid88in:enhance_logging into 7b5b602 on airbnb:master.

* SO-218: Reworked report edit panel * SO-218: Added Report Flyout to Dashboard * SO-218: Lint fix * Update superset-frontend/src/features/reports/ReportFlyout.tsx Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * SO-218: Fix import --------- Co-authored-by: Joey Andres <jandres@joeyandres.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

attempt to enchance logging

ef97c65

clean up logging

d1065ed

mistercrunch reviewed Apr 10, 2016
View reviewed changes

sidgupta added 3 commits April 10, 2016 11:05

clean up logging

06e8609

reset to logger

c6ea32d

clean up imports

58825e6

add comments in config.py

9452905

coleslaw reviewed Apr 10, 2016
View reviewed changes

remove redundant declaration of logging.config. Already exists in car…

50bb311

…avel/__init__.py

mistercrunch reviewed Apr 10, 2016
View reviewed changes

sidgupta added 3 commits April 10, 2016 16:45

replace RotatingFileHandler with TimedRotatingFileHandler

ad57252

revert back running web server in debug mode

49f27a4

fix debug in bin/caravel

e408332

resolve build errors - formatting

7f82849

revert back to default in master

e624c41

sid88in changed the title ~~attempt to enchance logging~~ enable timerotateloghandler (configurable) Apr 17, 2016

sidgupta added 2 commits April 16, 2016 18:40

fix build issues

de3475d

remove extra print statement

1f81a98

change log location to default

8cd0764

mistercrunch reviewed Apr 19, 2016
View reviewed changes

configure console log level and format

956c84d

Merge branch 'master' into enhance_logging

9f328e5

# Conflicts: # caravel/config.py # caravel/viz.py

mistercrunch merged commit d7ea473 into apache:master May 1, 2016

snyk-bot mentioned this pull request Jul 5, 2021

[Snyk] Fix for 1 vulnerabilities mysky528/incubator-superset#10

Open

mysky528 mentioned this pull request Dec 1, 2023

[Snyk] Security upgrade react-map-gl from 2.0.4 to 3.0.0 mysky528/incubator-superset#14

Open

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.9.0 First shipped in 0.9.0 labels Feb 19, 2024

Conversation

sid88in commented Apr 10, 2016

Uh oh!

coveralls commented Apr 10, 2016

Uh oh!

sid88in commented Apr 10, 2016

Uh oh!

mistercrunch Apr 10, 2016

Choose a reason for hiding this comment

Uh oh!

mistercrunch commented Apr 10, 2016

Uh oh!

sid88in commented Apr 10, 2016

Uh oh!

coveralls commented Apr 10, 2016

Uh oh!

coveralls commented Apr 10, 2016

Uh oh!

coleslaw Apr 10, 2016

Choose a reason for hiding this comment

Uh oh!

sid88in Apr 10, 2016

Choose a reason for hiding this comment

Uh oh!

coveralls commented Apr 10, 2016

Uh oh!

mistercrunch Apr 10, 2016

Choose a reason for hiding this comment

Uh oh!

sid88in Apr 10, 2016

Choose a reason for hiding this comment

Uh oh!

coveralls commented Apr 11, 2016

Uh oh!

coveralls commented Apr 11, 2016

Uh oh!

coveralls commented Apr 11, 2016

Uh oh!

landscape-bot commented Apr 11, 2016

Uh oh!

landscape-bot commented Apr 11, 2016

Uh oh!

landscape-bot commented Apr 11, 2016

Uh oh!

landscape-bot commented Apr 11, 2016

Uh oh!

coveralls commented Apr 11, 2016

Uh oh!

mistercrunch commented Apr 11, 2016

Uh oh!

coveralls commented Apr 17, 2016

Uh oh!

landscape-bot commented Apr 17, 2016

Uh oh!

sid88in commented Apr 17, 2016

Uh oh!

landscape-bot commented Apr 17, 2016

Uh oh!

landscape-bot commented Apr 17, 2016

Uh oh!

coveralls commented Apr 17, 2016

Uh oh!

landscape-bot commented Apr 17, 2016

Uh oh!

coveralls commented Apr 17, 2016

Uh oh!

coveralls commented Apr 17, 2016

Uh oh!

coveralls commented Apr 17, 2016

Uh oh!

coveralls commented Apr 17, 2016

Uh oh!

mistercrunch Apr 19, 2016

Choose a reason for hiding this comment

Uh oh!

sid88in Apr 19, 2016

Choose a reason for hiding this comment

Uh oh!

landscape-bot commented Apr 19, 2016

Uh oh!

coveralls commented May 1, 2016 •

edited

Loading