Replace print into logging #63

seeeturtle · 2017-12-18T15:00:31Z

This will replace print to standard logging.
It store logs in _site/community.log

Closes #9

manu-chroma · 2017-12-18T15:03:22Z

It should be Replace print with (in your commit)

Or rather a better short log would be: Use logging in place of print statements

seeeturtle · 2017-12-18T15:26:33Z

the log is empty in preview... what the matter?

jayvdb · 2017-12-18T15:35:11Z

log/view_log.py

+    return HttpResponse('<br>'.join(logs))
+
+def get_logs():
+    with open('./_site/community.log') as log_file:


https://deploy-preview-63--coala-community.netlify.com/static/community.log is empty ;-(

This is not how you serve a static file,
read https://docs.djangoproject.com/en/2.0/howto/static-files/

yukiisbored · 2017-12-19T11:47:56Z

community/decorators.py

@@ -0,0 +1,31 @@
+LOGGING_CONFIG = {


Hey, Django already setup logging for us, Please don't create a new one, just use the one Django already setup.
https://docs.djangoproject.com/en/2.0/topics/logging/

If you need to add more handlers, see https://docs.djangoproject.com/en/2.0/topics/logging/#configuring-logging,
Do it inside settings.py

yukiisbored · 2017-12-19T11:49:51Z

community/decorators.py

+}
+
+
+def load_logging_configuration(func):


Ref https://github.com/coala/community/pull/63/files#r157732719

jayvdb · 2017-12-20T08:01:22Z

Please change the log displayed to not include debug.
And squash your commits.

jayvdb · 2017-12-20T08:28:42Z

community/settings.py

+            'class': 'logging.FileHandler',
+            'filename': './_site/community.log',
+            'formatter': 'standard',
+            'level': 'NOTSET'


Set the level to INFO please.

why? we have some log with ERROR level. like here: https://github.com/seeeturtle/community/blob/af7306e3e886d38d230a50efafa790f7a26221cc/gci/config.py#L33

This specifies the minimum level, and all levels above that are included.

ah, I see. I didn't know well, sorry.

jayvdb

And squash your commits.

jayvdb · 2017-12-20T08:30:04Z

gci/client.py

@@ -18,14 +18,12 @@

 client = gciclient.GCIAPIClient(
      auth_token='xxxxxxxxxxxxxx',
-      url_prefix='https://codein.withgoogle.com',
-      debug=False)
+      url_prefix='https://codein.withgoogle.com')


this is not the problem. this is a docstring. please revert this bit.

jayvdb · 2017-12-20T08:30:34Z

gci/client.py

@@ -49,21 +47,13 @@ class GCIAPIClient(object):
  """

  def __init__(self, auth_token=None,
-               url_prefix='https://codein.withgoogle.com/',
-               debug=False):


revert this please. We still need debug support, just not enabled by default

jayvdb · 2017-12-20T08:33:14Z

gci/client.py

    self.url_prefix = urlparse.urljoin(url_prefix, 'api/program/current/')
    self.headers = {
        'Authorization': 'Bearer %s' % auth_token,
        'Content-Type': 'application/json',
    }

-    if debug:


please revert this. debug is not enabled. And is not enabled at https://github.com/coala/community/blob/master/gci/students.py#L30

yukiisbored · 2017-12-20T08:49:24Z

community/settings.py

+    'handlers': {
+        'communityHandler': {
+            'class': 'logging.FileHandler',
+            'filename': './_site/community.log',


Hey, put this inside the static folder.
https://docs.djangoproject.com/en/2.0/howto/static-files/

this isnt a pre-existing static file. It has to go in _site

jayvdb · 2017-12-20T13:00:32Z

gci/students.py

@@ -19,6 +20,7 @@
    'deadline',
 )

+


undo this unrelated change

jayvdb · 2017-12-20T13:00:45Z

gci/views.py

 import requests

 from .students import get_linked_students
 from .gitorg import get_logo
 from .task import get_tasks

+


undo this unrelated change

jayvdb · 2017-12-20T13:01:45Z

log/view_log.py

+def get_logs():
+    with open('./_site/community.log') as log_file:
+        for line in log_file:
+            if 'DEBUG' not in line:


no this is not how you filter the log. logging and django have official mechanisms for filtering the log.

We need things like access tokens to be never stored anywhere, at all. Leaving them lying around in files is begging for leaks to occur again due to mistakes.

ok. I'll follow the official mechanisms

jayvdb · 2017-12-20T13:50:07Z

gci/config.py

@@ -1,5 +1,7 @@
 import ruamel.yaml
 import os
+import logging
+


this extra blank line isnt desirable.

jayvdb · 2017-12-20T13:53:43Z

community/settings.py

@@ -11,6 +11,7 @@
 """

 import os
+from .filters import NoDebugFilter


add a blank line after system libraries, before local imports.

jayvdb · 2017-12-20T13:57:38Z

gci/config.py

@@ -27,6 +29,7 @@ def get_api_key(name):
            api_key = api_key_file.readline().strip()
            return api_key
    except IOError:
+        logger.error('Please put your %s API key at %s.' % (name, filename))


as a general rule, do not log before an exception.

An exception is telling the caller there was a problem, and the caller then has to decide what to do with the exception.
The caller may already know about this type of exception, and decide to ignore it because they do not consider it to be an error.

jayvdb · 2017-12-20T13:58:46Z

gci/config.py

@@ -15,6 +16,7 @@ class TokenMissing(KeyError):


 def get_api_key(name):
+    logger = logging.getLogger(__name__ + 'get_api_key')


logging names are dot separated. so this should be __name__ + '.get_api_key' . The same applies everywhere in your patch.

jayvdb · 2017-12-20T14:21:30Z

gci/gitorg.py

@@ -13,11 +14,12 @@
 class GitOrg():

    def __init__(self, name):
+        logger = logging.getLogger(__name__ + '__init__')


you need to add dots to all of the loggers.
And please rebase & squash before asking for reviews.

sorry, I missed it.

done @jayvdb

This will replace print statements with standard logging. It store logs in _site/community.log Closes coala#9

jayvdb · 2017-12-20T14:49:49Z

ack fec438a

gitmate-bot added process/pending review size/S difficulty/medium initiatives/gci labels Dec 18, 2017

seeeturtle force-pushed the issue-9 branch from 1238bc6 to da40d03 Compare December 18, 2017 15:18

jayvdb reviewed Dec 18, 2017

View reviewed changes

yukiisbored reviewed Dec 19, 2017

View reviewed changes

seeeturtle force-pushed the issue-9 branch from 9d02433 to 09613f5 Compare December 20, 2017 08:12

jayvdb requested changes Dec 20, 2017

View reviewed changes

yukiisbored reviewed Dec 20, 2017

View reviewed changes

seeeturtle force-pushed the issue-9 branch from 0762cab to af7306e Compare December 20, 2017 12:53

jayvdb requested changes Dec 20, 2017

View reviewed changes

seeeturtle force-pushed the issue-9 branch from 3dac0d6 to bd77140 Compare December 20, 2017 13:37

jayvdb reviewed Dec 20, 2017

View reviewed changes

gci/config.py Outdated

@@ -1,5 +1,7 @@

import ruamel.yaml

import os

import logging

Copy link

Member

jayvdb Dec 20, 2017

Choose a reason for hiding this comment

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

this extra blank line isnt desirable.

jayvdb reviewed Dec 20, 2017

View reviewed changes

seeeturtle force-pushed the issue-9 branch from 9351572 to 1e3f366 Compare December 20, 2017 13:55

jayvdb requested changes Dec 20, 2017

View reviewed changes

jayvdb reviewed Dec 20, 2017

View reviewed changes

seeeturtle force-pushed the issue-9 branch from c805c11 to 2432016 Compare December 20, 2017 14:27

Use logging in place of print statements

fec438a

This will replace print statements with standard logging. It store logs in _site/community.log Closes coala#9

seeeturtle force-pushed the issue-9 branch from 2432016 to fec438a Compare December 20, 2017 14:43

gitmate-bot added process/approved and removed process/pending review labels Dec 20, 2017

jayvdb merged commit fec438a into coala:master Dec 20, 2017

gitmate-bot added process/approved and removed process/approved labels Dec 20, 2017

jayvdb mentioned this pull request Jan 3, 2018

[WIP] Add generic GSoC 2017 import app #67

Closed

sks444 mentioned this pull request Apr 13, 2018

Use logging sks444/awesome100#9

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace print into logging #63

Replace print into logging #63

seeeturtle commented Dec 18, 2017

manu-chroma commented Dec 18, 2017 •

edited

Loading

seeeturtle commented Dec 18, 2017

jayvdb Dec 18, 2017

yukiisbored Dec 20, 2017

yukiisbored Dec 19, 2017

yukiisbored Dec 19, 2017

yukiisbored Dec 19, 2017

jayvdb commented Dec 20, 2017

jayvdb Dec 20, 2017

seeeturtle Dec 20, 2017

jayvdb Dec 20, 2017

seeeturtle Dec 20, 2017

jayvdb left a comment

jayvdb Dec 20, 2017

jayvdb Dec 20, 2017

jayvdb Dec 20, 2017

yukiisbored Dec 20, 2017

jayvdb Dec 20, 2017

jayvdb Dec 20, 2017

jayvdb Dec 20, 2017

jayvdb Dec 20, 2017

seeeturtle Dec 20, 2017

jayvdb Dec 20, 2017

jayvdb Dec 20, 2017

jayvdb Dec 20, 2017

seeeturtle Dec 20, 2017

jayvdb Dec 20, 2017

seeeturtle Dec 20, 2017

jayvdb Dec 20, 2017 •

edited

Loading

seeeturtle Dec 20, 2017

seeeturtle Dec 20, 2017

jayvdb commented Dec 20, 2017

		@@ -15,6 +16,7 @@ class TokenMissing(KeyError):


		def get_api_key(name):
		logger = logging.getLogger(__name__ + 'get_api_key')

Replace print into logging #63

Replace print into logging #63

Conversation

seeeturtle commented Dec 18, 2017

manu-chroma commented Dec 18, 2017 • edited Loading

seeeturtle commented Dec 18, 2017

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

jayvdb commented Dec 20, 2017

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

jayvdb left a 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

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

jayvdb Dec 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jayvdb commented Dec 20, 2017

manu-chroma commented Dec 18, 2017 •

edited

Loading

jayvdb Dec 20, 2017 •

edited

Loading