[chirp_devel] [PATCH 1/7] Add chirp.logger module (#2347)

[Zach Welch]

On 02/26/2015 04:18 PM, Dan Smith wrote:
>> # HG changeset patch
>> # User Zach Welch <zach at mandolincreekfarm.com>
> 
> Can you put the git hash into this block? When people send multiple
> copies of the same patch before I get far enough to run the patchbot on
> them, I use the NodeID to make sure I'm applying the thing I'm reviewing
> in my mailer. The git hash would give me that for your patches as well.

I will see what I can do.  It does not appear anywhere in the original
patches, and I don't see an option that will add it.  A quick web search
also failed to turn up a solution.  I can make it happen, but it may
take a little creativity (and time) on my part.

That said, I could just fake it, since it's apples to oranges anyway. As
long as it's unique for each patch, it should serve for the purpose that
you describe, yah?  That would be fairly easy to add; in the
git-patch-to-hg-patch script, hash the original email message (which
includes a time/date string) and then emit it as part of the body.

>> +#: Map human-readable logging levels to their internal values.
>> +log_level_names = {"critical": logging.CRITICAL,
>> +                   "error":    logging.ERROR,
>> +                   "warn":     logging.WARNING,
>> +                   "info":     logging.INFO,
>> +                   "debug":    logging.DEBUG,
>> +                   }
> 
> My point in the other patch was that you shouldn't replicate the level
> names here when you can get them from the library. But I think the point
> here is to let you do CHIRP_DEBUG=info, which means they needn't be the
> canonical strings anyway, so, that's fine.

That's the point, yes.  Again, I couldn't find a "getLevelForName"
method in the logger module, or I would have used it instead.

>> +    def create_log_file(self, name):
>> +        if self.logfile is None:
>> +            self.logname = name
>> +            lf = file(name, "w")
>> +            print >>lf, version_string()
>> +            lf.close()
> 
> What is the point of writing the raw string into the file before we open
> it? That said, the above should be this for consistency:

Well, it works.  The string does make it into the log file.

>  with file(name, 'w') as lf:
>     print >>lf, version_string()
> 
> Which auto-closes the file. Since this is at the bottom of the stack,
> I'll apply, but I think we should go back and just remove that (or turn
> it into a log), unless there is some reason I'm missing.

I was unaware of that convention.  As it was, I copy-and-pasted that
code from setup.py, so I'll fix it up at the same time as the above.

It's not a log message because that would end up on the console as well,
and we should not unilaterally spam that line at every launch (as that
could preclude chirpc from being usable in command pipelines).

-- 
Zach Welch
Mandolin Creek Farm
www.mandolincreekfarm.com
farm: 541-453-4131
cell: 541-740-3410