UniversalDevicesInc / Polyglot

Polyglot Open Source
MIT License
24 stars 2 forks source link

Proper method to send info to the log #18

Closed jimboca closed 8 years ago

jimboca commented 8 years ago

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?

Einstein42 commented 8 years ago

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.

jimboca commented 8 years ago

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.

Einstein42 commented 8 years ago

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

Einstein42 commented 8 years ago

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

jimboca commented 8 years ago

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.

Einstein42 commented 8 years ago

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

jimboca commented 8 years ago

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 :)

Einstein42 commented 8 years ago

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

jimboca commented 8 years ago

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?

jimboca commented 8 years ago

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

jimboca commented 8 years ago

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.

Einstein42 commented 8 years ago

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

mkohanim commented 8 years ago

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/UniversalDevicesInc/Polyglot/issues/18#issuecomment-201281667

Einstein42 commented 8 years ago

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

Einstein42 commented 8 years ago

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?

jimboca commented 8 years ago

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 https://github.com/UniversalDevicesInc/Polyglot/issues/18#issuecomment-202610456

Einstein42 commented 8 years ago

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.

jimboca commented 8 years ago

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 https://github.com/UniversalDevicesInc/Polyglot/issues/18#issuecomment-202618124

Einstein42 commented 8 years ago

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()
jimboca commented 8 years ago

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 https://github.com/UniversalDevicesInc/Polyglot/issues/18#issuecomment-202619629

Einstein42 commented 8 years ago

Use self.poly.LOGGER.('')

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

Einstein42 commented 8 years ago

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

Einstein42 commented 8 years ago

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

jimboca commented 8 years ago

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

Einstein42 commented 8 years ago

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.

Einstein42 commented 8 years ago

Documentation is complete on the Wiki.

mkohanim commented 8 years ago

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/UniversalDevicesInc/Polyglot/issues/18#issuecomment-205366909

Einstein42 commented 8 years ago

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.

jimboca commented 8 years ago

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...

mkohanim commented 8 years ago

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/UniversalDevicesInc/Polyglot/issues/18#issuecomment-210256226

jimboca commented 8 years ago

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

jimboca commented 8 years ago

Changes merged fro #44