libtaskotron/__init__.py
4 ↗	(On Diff #13)	In general, init on import is a bad idea. I think this should be initialized on startup of the runner instead. Explicit is better in this case and it'll be all the more important when we start shipping logs - it shouldn't try to connect to anything over the network until explicitly initialized
libtaskotron/runner.py
3	Can you back the logging changes out of the runner? I'd prefer adding these changes to D5 or separately so that the merge into that code isn't quite so messy since D5 changes this code so much.

libtaskotron/__init__.py
4 ↗	(On Diff #13)	Just a note, the init function will not change when we start shipping logs nor it will try to connect to anything over the network. logstash just scans whatever logging produces. Anyway, I am fine with putting init into the runner.
libtaskotron/runner.py
3	Yeah, I was/am going to hold on with this until D5 is merged in develop.

libtaskotron/__init__.py
4 ↗	(On Diff #13)	How will it work when I use some libtaskotron module but don't run it through the runner (containing the init code)? Martin, please test.
4 ↗	(On Diff #13)	Do we want to default to DEBUG? It might be better to have a command line option for that a default to WARN or INFO?
libtaskotron/file_utils.py
77	Let's change to logging.debug()
libtaskotron/logger.py
37–38	Can you add docstrings for the arguments? Not everyone knows what 'streaming' is, where does it stream to, and what kind of values filelog accepts - e.g. file names or handlers. Should filelog default to None instead?

libtaskotron/__init__.py
4 ↗	(On Diff #13)	How will it work when I use some libtaskotron module but don't run it through the runner (containing the init code)? Martin, please test. In that case, the output is muted. You need to run the init function in order to use logging. Do we want to default to DEBUG? It might be better to have a command line option for that a default to WARN or INFO? Let's leave default to DEBUG for now for development purposes and change it along with adding command line/configuration options with sane defaults for another task.

libtaskotron/directives/koji_directive.py
83	pylint says you can use comma instead of the percent sign, it could be a bit more efficient. this applies to everywhere else
libtaskotron/logger.py
35	The docs say you can use logger.exception() for this use case: http://docs.python.org/2.7/library/logging.html#logging.Logger.exception Also, you should call this on 'log' variable instead of 'logging' module?
39	I think this should go into init() (i.e. we run the program)? Otherwise we will override this setting for anyone using our library.
44	I would default to name='libtaskotron'
libtaskotron/runner.py
126	libtaskotron.logger.init() would be clearer here

		Path
M		libtaskotron/directives/koji_directive.py (8 lines)
M		libtaskotron/file_utils.py (9 lines)
A	M	libtaskotron/logger.py (77 lines)
M		libtaskotron/runner.py (7 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	13	a1de947		Feb 11 2014, 3:59 PM	★	★
Diff 2	15	a1de947	- Move logger init into runner	Feb 12 2014, 12:54 PM	★	★
Diff 3	17	a1de947	- Add docstring to the logger.init function	Feb 12 2014, 2:43 PM	★	★
Diff 4	19	a1de947	- Use custom logger instead of the root logger	Feb 19 2014, 12:23 PM	★	★
Diff 5	20	a1de947	- Move excepthook into logger.init	Feb 19 2014, 3:30 PM	★	★
Diff 6	22	3fb8c35	Commit rLTRN2a5dc7b7e7b87f496f54652cbbf570f2edc42158	Feb 20 2014, 9:55 AM	★	★

Commit	Tree	Parents	Author	Summary	Date
7094da3aaa06	99f6ab92758e	cf8d4a75b32c	Martin Krizek	Move excepthook into logger.init	Feb 19 2014, 3:30 PM
cf8d4a75b32c	4d80f3d07383	39815083e971	Martin Krizek	Use custom logger instead of the root logger	Feb 19 2014, 12:22 PM
39815083e971	af40cc625c04	06cf1a51f269	Martin Krizek	Add docstring to the logger.init function	Feb 12 2014, 2:42 PM
06cf1a51f269	98f889a678fa	884f665e62e0	Martin Krizek	Move logger init into runner	Feb 12 2014, 12:52 PM
884f665e62e0	da846a7f19c0	a1de947a8136	Martin Krizek	Add logging support (Show More…)	Feb 11 2014, 3:44 PM

Diff 22

libtaskotron/directives/koji_directive.py

1	import koji	1		import koji
2		2
3	from libtaskotron import file_utils	3		from libtaskotron import file_utils
		4		from libtaskotron.logger import log
4	from libtaskotron.directives import BaseDirective	5		from libtaskotron.directives import BaseDirective
5		6
6	directive_class = 'KojiDirective'	7		directive_class = 'KojiDirective'
7		8
8	koji_url = 'http://koji.fedoraproject.org/kojihub'	9		koji_url = 'http://koji.fedoraproject.org/kojihub'
9	pkg_url = 'http://kojipkgs.fedoraproject.org/packages'	10		pkg_url = 'http://kojipkgs.fedoraproject.org/packages'
10		11
11		12
▲ Show 20 Lines • Show All 62 Lines • ▼ Show 20 Line(s)		67	def get_nvr_rpms(self, nvr, rpm_dir, arches=None, debuginfo=False, src=False):
74	'''	75		'''
75		76
76	rpm_urls = self.nvr_to_urls(nvr,	77		rpm_urls = self.nvr_to_urls(nvr,
77	arches=arches,	78		arches=arches,
78	debuginfo=debuginfo,	79		debuginfo=debuginfo,
79	src=src)	80		src=src)
80		81
81	rpm_files = []	82		rpm_files = []
82	print 'Fetching RPMs for: %s' % nvr	83		log.info('Fetching RPMs for: %s', nvr)
			kparalUnsubmitted Not Done pylint says you can use comma instead of the percent sign, it could be a bit more efficient. this applies to everywhere else
83	for url in rpm_urls:	84		for url in rpm_urls:
84	print ' fetching %s' % url	85		log.info(' fetching %s', url)
85	rpm_file = file_utils. download(url, rpm_dir)	86		rpm_file = file_utils. download(url, rpm_dir)
86	rpm_files.append(rpm_file)	87		rpm_files.append(rpm_file)
87		88
88	return rpm_files	89		return rpm_files
89		90
90	def process(self, input_data, env_data):	91		def process(self, input_data, env_data):
91	valid_actions = ['download']	92		valid_actions = ['download']
92		93
93	output_data = {}	94		output_data = {}
94		95
95	action = input_data['action']	96		action = input_data['action']
96		97
97	if not action in valid_actions:	98		if not action in valid_actions:
98	raise Exception('%s is not a valid command for koji helper' % action)	99		raise Exception('%s is not a valid command for koji helper' % action)
99		100
100	# for now, the only valid command is download, can simplify code	101		# for now, the only valid command is download, can simplify code
101		102
102	if action == 'download':	103		if action == 'download':
103	if 'envr' in input_data and 'arch' in input_data:	104		if 'envr' in input_data and 'arch' in input_data:
104	# we need to find the tmpdir and nvr from input data	105		# we need to find the tmpdir and nvr from input data
105	tmpdir = env_data['workdir']	106		tmpdir = env_data['workdir']
106	envr = input_data['envr']	107		envr = input_data['envr']
107	arches = [input_data['arch']]	108		arches = [input_data['arch']]
108		109
109	print "getting koji builds for %s (%s) and downloading to %s" %\	110		log.info("getting koji builds for %s (%s) and downloading to %s", envr, arches, tmpdir)
110	(envr, arches, tmpdir)
111	output_data['downloaded_rpms'] = self.get_nvr_rpms(envr,	111		output_data['downloaded_rpms'] = self.get_nvr_rpms(envr,
112	tmpdir,	112		tmpdir,
113	arches)	113		arches)
114		114
115	return output_data	115		return output_data

View Options

libtaskotron/file_utils.py

1	import os	1		import os
2	import sys	2		import sys
3	import urlgrabber.grabber, urlgrabber.progress	3		import urlgrabber.grabber, urlgrabber.progress
		4		from libtaskotron.logger import log
4		5
5		6
6	def makedirs(fullpath):	7		def makedirs(fullpath):
7	'''This is the same as @method os.makedirs(), but does not throw an	8		'''This is the same as @method os.makedirs(), but does not throw an
8	exception when the destination directory already exists.'''	9		exception when the destination directory already exists.'''
9	try:	10		try:
10	os.makedirs(fullpath)	11		os.makedirs(fullpath)
11	assert os.path.isdir(fullpath)	12		assert os.path.isdir(fullpath)
▲ Show 20 Lines • Show All 56 Lines • ▼ Show 20 Line(s)		46	def download(url, dest, overwrite=False, grabber=None):
68	else:	69		else:
69	makedirs(os.path.dirname(dest))	70		makedirs(os.path.dirname(dest))
70		71
71	# overwrite?	72		# overwrite?
72	if os.path.exists(dest):	73		if os.path.exists(dest):
73	if overwrite:	74		if overwrite:
74	os.remove(dest)	75		os.remove(dest)
75	else:	76		else:
76	print 'Already downloaded: %s' % os.path.basename(dest)	77		log.debug('Already downloaded: %s', os.path.basename(dest))
			kparalUnsubmitted Not Done Let's change to logging.debug()
77	return dest	78		return dest
78		79
79	print 'Downloading: %s' % url	80		log.info('Downloading: %s', url)
80	try:	81		try:
81	grabber.urlgrab(url, dest)	82		grabber.urlgrab(url, dest)
82	except urlgrabber.grabber.URLGrabError, e:	83		except urlgrabber.grabber.URLGrabError, e:
83	print >> sys.stderr, 'Download error: %s' % e	84		log.error('Download error: %s', e)
84	# the file can be incomplete, remove	85		# the file can be incomplete, remove
85	if os.path.exists(dest):	86		if os.path.exists(dest):
86	try:	87		try:
87	os.remove(dest)	88		os.remove(dest)
88	except:	89		except:
89	print >> sys.stderr, 'Could not delete incomplete file: %s' % dest	90		log.error('Could not delete incomplete file: %s', dest)
90	raise	91		raise
91		92
92	return dest	93		return dest
93		94

View Options

libtaskotron/logger.py

This file was added.

1		#!/usr/bin/python
2		#
3		# Copyright 2014, Red Hat, Inc.
4		#
5		# This program is free software; you can redistribute it and/or modify
6		# it under the terms of the GNU General Public License as published by
7		# the Free Software Foundation; either version 2 of the License, or
8		# (at your option) any later version.
9		#
10		# This program is distributed in the hope that it will be useful,
11		# but WITHOUT ANY WARRANTY; without even the implied warranty of
12		# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13		# GNU General Public License for more details.
14		#
15		# You should have received a copy of the GNU General Public License along
16		# with this program; if not, write to the Free Software Foundation, Inc.,
17		# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
18		#
19		# Author: Martin Krizek <mkrizek@redhat.com>
20
21		import sys
22		import logging
23		import logging.handlers
24		import traceback
25
26
27		# http://docs.python.org/2/howto/logging.html#configuring-logging-for-a-library
28		log = logging.getLogger('libtaskotron')
29		log.addHandler(logging.NullHandler())
30
31
32		def _log_excepthook(*exc_info):
33		'''Called when an exception is not caught'''
34		log.critical(''.join(traceback.format_exception(*exc_info)))
35
	kparalUnsubmitted Not Done The docs say you can use logger.exception() for this use case: http://docs.python.org/2.7/library/logging.html#logging.Logger.exception Also, you should call this on 'log' variable instead of 'logging' module?
36
37		def init(name='libtaskotron', level=logging.INFO, stream=True, syslog=False, filelog=None):
38		"""Setup a logger
	kparalUnsubmitted Not Done Can you add docstrings for the arguments? Not everyone knows what 'streaming' is, where does it stream to, and what kind of values filelog accepts - e.g. file names or handlers. Should filelog default to None instead?
39		@param name name of the logger, 'libtaskotron' is default
	kparalUnsubmitted Not Done I think this should go into init() (i.e. we run the program)? Otherwise we will override this setting for anyone using our library.
40		@param level level of logging
41		@param stream enables logging to stderr
42		@param syslog enables logging to syslog
43		@param filelog enables logging to a file, the value is the file name
44		"""
	kparalUnsubmitted Not Done I would default to name='libtaskotron'
45
46		# overwrite default except hook
47		sys.excepthook = _log_excepthook
48
49		logger = logging.getLogger(name)
50		logger.setLevel(level)
51		logger.addHandler(logging.NullHandler())
52
53		fmt = '[%(name)s:%(filename)s:%(lineno)d] %(asctime)s %(levelname)-7s %(message)s'
54		datefmt = '%Y-%m-%d %H:%M:%S'
55		formatter = logging.Formatter(fmt=fmt, datefmt=datefmt)
56
57		if stream:
58		stream_handler = logging.StreamHandler()
59		stream_handler.setFormatter(formatter)
60		logger.addHandler(stream_handler)
61		logger.debug("doing stream logging")
62
63		if syslog:
64		syslog_handler = logging.handlers.SysLogHandler(
65		address='/dev/log', facility=logging.handlers.SysLogHandler.LOG_LOCAL4)
66		syslog_handler.setFormatter(formatter)
67		logger.addHandler(syslog_handler)
68		logger.debug("doing syslog logging")
69
70		if filelog:
71		file_handler = logging.handlers.RotatingFileHandler(filelog,
72		maxBytes=500000, backupCount=5)
73		file_handler.setFormatter(formatter)
74		logger.addHandler(file_handler)
75		logger.debug('doing file logging to %s' % filelog)
76
77		return logger

View Options

libtaskotron/runner.py

		1		import logging
1	import tempfile	2		import tempfile
2	import os.path	3		import os.path
			tflinkUnsubmitted Not Done Can you back the logging changes out of the runner? I'd prefer adding these changes to D5 or separately so that the merge into that code isn't quite so messy since D5 changes this code so much.
			mkrizekAuthorUnsubmitted Not Done Yeah, I was/am going to hold on with this until D5 is merged in develop.
3	import argparse	4		import argparse
4	import imp	5		import imp
5	from jinja2 import Template, Environment	6		from jinja2 import Template, Environment
6	import copy	7		import copy
7		8
8	from libtaskotron import taskyaml	9		from libtaskotron import taskyaml
		10		from libtaskotron import logger
		11		from libtaskotron.logger import log
9		12
10	class Runner:	13		class Runner:
11	def __init__(self, taskdata, argdata, workdir=None):	14		def __init__(self, taskdata, argdata, workdir=None):
12	self.taskdata = taskdata	15		self.taskdata = taskdata
13	self.envdata = argdata	16		self.envdata = argdata
14	self.working_data = {}	17		self.working_data = {}
15	self.directives = {}	18		self.directives = {}
16	self.jinja_env = None	19		self.jinja_env = None
▲ Show 20 Lines • Show All 98 Lines • ▼ Show 20 Line(s)		115	parser.add_argument("-a", "--arch", choices=["i386", "x86_64", "armhfp", "noarch"], help="""
115	""")	118		""")
116	parser.add_argument("-e", "--envr", help="envr to be used as input")	119		parser.add_argument("-e", "--envr", help="envr to be used as input")
117	return parser	120		return parser
118		121
119	def main():	122		def main():
120	parser = get_argparser()	123		parser = get_argparser()
121	args = parser.parse_args()	124		args = parser.parse_args()
122		125
		126		logger.init(level=logging.DEBUG)
			kparalUnsubmitted Not Done libtaskotron.logger.init() would be clearer here
		127
123	arg_data = vars(args)	128		arg_data = vars(args)
124	arg_data['taskfile'] = args.task[0]	129		arg_data['taskfile'] = args.task[0]
125		130
126	task_data = taskyaml.parse_yaml_from_file(arg_data['taskfile'])	131		task_data = taskyaml.parse_yaml_from_file(arg_data['taskfile'])
127		132
128	task_runner = Runner(task_data, arg_data)	133		task_runner = Runner(task_data, arg_data)
129	task_runner.run()	134		task_runner.run()
130		135
131	for output in task_runner.working_data:	136		for output in task_runner.working_data:
132	print task_runner.working_data[output]	137		log.info(task_runner.working_data[output])
133	No newline at end of file

Add logging support
ClosedPublic
Actions

Details

Diff Detail

Revision Contents

Diff 22

libtaskotron/directives/koji_directive.py

libtaskotron/file_utils.py

libtaskotron/logger.py

libtaskotron/runner.py

Add logging supportClosedPublicActions

Details

Diff Detail

Revision Contents

Diff 22

libtaskotron/directives/koji_directive.py

libtaskotron/file_utils.py

libtaskotron/logger.py

libtaskotron/runner.py

Add logging support
ClosedPublic
Actions