UniversalDevicesInc/Polyglot

Proper method to send info to the log

Closed this issue · 32 comments

Polyglot includes a send_error method which looks like it will eventually pass an error back to the ISY for it to report. But, what is the standard way to add info and warnings to the log from a Node or NodeServer? I tried doing my own getlogger call but that doesn't seem to work?

I would assume that nodes and nodeservers should define their own log files and not log to the root LOGGER. I have a nice one I've used for a while if you'd like to have it.

Yes, I thought about that but when we need to debug user issues it would be easier to have the user send the polyglot log instead of needing both logs to see what is happening?

But please do send me what you are doing for now.

import logging
import logging.handlers
import time
import sys
import os

def setup_log(name):

Log Location

LOG_FILENAME = os.path.dirname(os.path.abspath(file)) + "/myq.log"
LOG_LEVEL = logging.INFO # Could be e.g. "DEBUG" or "WARNING"

Logging Section

LOGGER = logging.getLogger(name)
LOGGER.setLevel(LOG_LEVEL)

Set the log level to LOG_LEVEL

Make a handler that writes to a file,

making a new file at midnight and keeping 30 backups

HANDLER = logging.handlers.TimedRotatingFileHandler(LOG_FILENAME, when="midnight", backupCount=30)

Format each log message like this

FORMATTER = logging.Formatter('%(asctime)s %(levelname)-8s %(message)s')

Attach the formatter to the handler

HANDLER.setFormatter(FORMATTER)

Attach the handler to the logger

LOGGER.addHandler(HANDLER)
return LOGGER

I use the previous comment as 'logger.py' and in main I do

from logger import setup_log

LOGGER = setup_log('polyglot_myq')

LOGGER.info('blah is set to %s', blah)

Then just pass logger to any sub modules

Thanks! that will work for now. But would also be nice to get the path to the polyglot/config/nodeserver to store the log instead of where the source is located.

that logger.py is in that polyglot/config/nodeserver/myq/ folder so that is where my myq.log file is generated.

Yes, but I meant the config/ directory not config/node_severs/ because I didn't want the log file showing up in the same directory as the source. Maybe we can assume ../../ but that might not work for me because I create a softlink to my actual source directory.

It would be nice if polyglot had a poly.config_dir for the path to the top of the config dir it is currently using? Maybe that can be added easily since you are familiar with that part of the code :)

Yea that shouldn't be a problem. You just want the root /Polyglot/config directory right?

The root would be good, but might be best to get the config/ directory since it looks like Polyglot always creates that directory for you?

Of course github messed with my comment, I said the config/nodeservername directory.

Using @Einstein42 suggested code, I now do this:

#
# File: logger.py
#
# Create a log for the nodeserver. In the setup method call with:
#   self.logger = setup_log(self.poly.sandbox,self.poly.name)
#

import logging
import logging.handlers
import time

def setup_log(path,name):
   # Log Location
   LOG_FILENAME = path + "/" + name + ".log"
   LOG_LEVEL = logging.DEBUG  # Could be e.g. "DEBUG" or "WARNING"

   #### Logging Section ################################################################################
   LOGGER = logging.getLogger(name)
   LOGGER.setLevel(LOG_LEVEL)
   # Set the log level to LOG_LEVEL
   # Make a handler that writes to a file, 
   # making a new file at midnight and keeping 30 backups
   HANDLER = logging.handlers.TimedRotatingFileHandler(LOG_FILENAME, when="midnight", backupCount=30)
   # Format each log message like this
   FORMATTER = logging.Formatter('%(asctime)s %(levelname)-8s %(message)s')
   # Attach the formatter to the handler
   HANDLER.setFormatter(FORMATTER)
   # Attach the handler to the logger
   LOGGER.addHandler(HANDLER)
   return LOGGER

IMO, this code should be part of the NodeServer, so all Polyglot servers put the log in the same place.

Agreed. I'm ok with making this a server side change. Good thought.

Hello all,

I also agree!

With kind regards,


Michel Kohanim
CEO

(p) 818.631.0333
(f) 818.436.0702
http://www.universal-devices.comhttp://www.universal-devices.com/


From: James [mailto:notifications@github.com]
Sent: Friday, March 25, 2016 6:39 AM
To: UniversalDevicesInc/Polyglot Polyglot@noreply.github.com
Subject: Re: [Polyglot] Proper method to send info to the log (#18)

Agreed. I'm ok with making this a server side change. Good thought.


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHubhttps://github.com//issues/18#issuecomment-201281667

Did you add this into the nodeserver_api yet? I was going to look at working on this maybe later today if not.

I'm having an issue with this. If I put it in the nodeserver_api then I have to wait for the params STDIN, which I moved before the config from the nodeserver_manager, so that I can setup during the poly.wait_for_config() set in a normal client nodeserver startup. The problem I'm running into is that I can't log anything in the init of the SimpleNodeServer(NodeServer) because the params haven't been sent from nodeserver_manager yet when the SimpleNodeServer is instantiated. So if you do anything in init override you wont have a logger. Thoughts?

If I am understanding correctly, I think this is okay, since typically you
want need to override the NodeServer init method?

On Mon, Mar 28, 2016 at 3:41 PM James notifications@github.com wrote:

I'm having an issue with this. If I put it in the nodeserver_api then I
have to wait for the params STDIN, which I moved before the config from the
nodeserver_manager, so that I can setup during the poly.wait_for_config()
set in a normal client nodeserver startup. The problem I'm running into is
that I can't log anything in the init of the
SimpleNodeServer(NodeServer) because the config hasn't been sent yet. So if
you do a lot of things in init override you wont have a logger.
Thoughts?


You are receiving this because you were assigned.
Reply to this email directly or view it on GitHub
#18 (comment)

You mean you DONT typically want to override the NodeServer init? I agree, but some people might want to super and then initialize a connection or something. I guess I just need to document that and make sure that I catch any errors that might occur by doing that.

Sorry, yes, typo. Typically you don't override the init method. If you
do, then you have to assume the logger is not initialized until after your
super call.

On Mon, Mar 28, 2016 at 4:05 PM James notifications@github.com wrote:

You mean you DONT typically want to override the NodeServer init? I agree,
but some people might want to super and then initialize a connection or
something. I guess I just need to document that and make sure that I catch
any errors that might occur by doing that.


You are receiving this because you were assigned.
Reply to this email directly or view it on GitHub
#18 (comment)

Well it wouldn't even be initialized then. It won't be initialized until the config is received from the nodeserver_manager. Basically this is the normal startup procedure:

  1. poly = PolyglotConnector()
  2. nserver = ExampleSimpleNodeServer(SimpleNodeServer) <--- this would be where they could override init
  3. poly.connect()
  4. poly.wait_for_config() < --- this is where the logger is finally instantiated
  5. nserver.setup()
  6. nserver.run()

Ok, I see. Then, I am not sure what is best at the moment, I can try to
think about it more tonight.

On Mon, Mar 28, 2016 at 4:10 PM James notifications@github.com wrote:

Well it wouldn't even be initialized then. It won't be initialized until
the config is received from the nodeserver_manager. Basically this is the
normal startup procedure:

  1. poly = PolyglotConnector()
  2. nserver = ExampleSimpleNodeServer(SimpleNodeServer) <--- this would
    be where they could override init
  3. poly.connect()
  4. poly.wait_for_config() < --- this is where the logger is finally
    instantiated
  5. nserver.setup()
  6. nserver.run()


You are receiving this because you were assigned.
Reply to this email directly or view it on GitHub
#18 (comment)

Use self.poly.LOGGER.('')

eg: self.poly.LOGGER.info("FROM Poly ISYVER: " + self.poly.isyver)

On a side note. I've had a hell of a time passing vars from triple nested classes.

This has been working well for me. Does anyone else need anything on this?

I have not been able to try it while out of town, but it sounds perfect. Did you add it to the documentation?

No. I have been adding all my documentation to the 'wiki' on github. But I do need to add a bunch more. We are going to have to figure out who's going to get the fun task of updating all the existing stuff and putting it on the wiki. 1 2 3 not it.

Documentation is complete on the Wiki.

Thanks James.

Is this automatically updated in the docs folder? There’s not much on the Wiki … I do not mind doing the documentation but I need to know what I’d be updating.

With kind regards,


Michel Kohanim
CEO

(p) 818.631.0333
(f) 818.436.0702
http://www.universal-devices.comhttp://www.universal-devices.com/


From: James [mailto:notifications@github.com]
Sent: Monday, April 4, 2016 8:59 AM
To: UniversalDevicesInc/Polyglot Polyglot@noreply.github.com
Cc: Michel Kohanim michel@universal-devices.com
Subject: Re: [UniversalDevicesInc/Polyglot] Proper method to send info to the log (#18)

Documentation is complete on the Wiki.


You are receiving this because you commented.
Reply to this email directly or view it on GitHubhttps://github.com//issues/18#issuecomment-205366909

I don't know how to update the docs in that folder. I'd be happy to, but I'm not familiar with the format.

I've switched to this and it looks great, thanks!

Comments:

  1. Do we want this to be LOGGER? Not sure why it needs to be uppercase?
  2. Should we just add the .logger to SimpleNodeServer instead of each one having to do it? But, it can't be in the init, so it has to be in setup, and SimpleNodeServer doesn't have a setup that each node server runs via super, but maybe it should?
  3. I would like the filename to be accesable so I can throw an exception and tell the user where to look: raise ValueError('Error in config file "%s", see log "%s"' % (self.config_file, self.poly.log_filename))

I can look at updating the docs since I need to learn how that works for my changes...

I think so!

With kind regards,


Michel Kohanim
CEO

(p) 818.631.0333
(f) 818.436.0702
http://www.universal-devices.comhttp://www.universal-devices.com/


From: jimboca [mailto:notifications@github.com]
Sent: Thursday, April 14, 2016 7:25 PM
To: UniversalDevicesInc/Polyglot Polyglot@noreply.github.com
Cc: Michel Kohanim michel@universal-devices.com
Subject: Re: [UniversalDevicesInc/Polyglot] Proper method to send info to the log (#18)

I've switched to this and it looks great, thanks!

Should we just add the .logger to SimpleNodeServer instead of each one having to do it?

I can look at updating the docs since I need to learn how that works for my changes...


You are receiving this because you commented.
Reply to this email directly or view it on GitHubhttps://github.com//issues/18#issuecomment-210256226

This is done and documented. Can be closed when #44 is approved by someone and merged.

Changes merged fro #44