file_utils.download now accepts dirname and filename separately
ClosedPublic
Actions

Authored by lbrabec on Jun 24 2014, 9:11 AM.

Details

Reviewers

tflink
jskladan

Maniphest Tasks

T218: Fix dir vs. file confusion in file_utils.py/download()

Commits

rLTRNedfb3abcda1d: file_utils.download now accepts dirname and filename separately

Summary

Argument dest was replaced with separate dirname and filename (WIP).

Test Plan

WIP, will be discussed with @jskladan

Diff Detail

Repository

rLTRN libtaskotron

Lint

Lint Skipped

Unit

Unit Tests Skipped

lbrabec retitled this revision from to file_utils.download now accepts dirname and filename separately.Jun 24 2014, 9:11 AM

lbrabec updated this object.

lbrabec edited the test plan for this revision. (Show Details)

lbrabec added a reviewer: jskladan.

lbrabec added a subscriber: jskladan.

Herald added a subscriber: tflink. · View Herald TranscriptJun 24 2014, 9:11 AM

lbrabec added a task: T218: Fix dir vs. file confusion in file_utils.py/download().Jun 24 2014, 9:11 AM

Although I'd like to see some tests, mocking filesystem is not that straightforward (that probably being the reason for zero coverage of file_utils), so I guess that we'll have to do without these. Or is a "functional" test creating (and the cleaning up) some tmpdir a viable option? Adding @tflink as the testing guru ;)

Otherwise, the code looks good to me.

Time passes, I guess you can ship it...

This revision is now accepted and ready to land.Jun 25 2014, 1:15 PM

Closed by commit rLTRNedfb3abcda1d (authored by @lbrabec).

Revision Contents

			Path	Packages
M			libtaskotron/file_utils.py (30 lines)
M			libtaskotron/koji_utils.py (27 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	451	1e338d6		Jun 24 2014, 9:08 AM	★	★
Diff 2	461	1e338d6	Commit rLTRNedfb3abcda1de760038bf3a97dca1c1f02dc89b2	Jun 24 2014, 9:07 AM	★	★

Commit	Tree	Parents	Author	Summary	Date
edfb3abcda1d	4fa97eaeba63	1e338d679a13	Lukas Brabec	file_utils.download now accepts dirname and filename separately	Jun 24 2014, 8:52 AM

Diff 461

View Options

libtaskotron/file_utils.py

Show First 20 Lines • Show All 50 Lines • ▼ Show 20 Line(s)		35	def get_urlgrabber(**kwargs):
51		51
52	# retry on socket timeout	52		# retry on socket timeout
53	if 12 not in grabber.opts.retrycodes:	53		if 12 not in grabber.opts.retrycodes:
54	grabber.opts.retrycodes.append(12)	54		grabber.opts.retrycodes.append(12)
55		55
56	return grabber	56		return grabber
57		57
58		58
59	def download(url, dest, overwrite=False, grabber=None):	59		def download(url, dirname, filename=None, overwrite=False, grabber=None):
60	'''Download a file.	60		'''Download a file.
61		61
62	:param str url: file URL to download	62		:param str url: file URL to download
63	:param str dest: either an existing directory or a file path. In the latter	63		:param str dirname directory path, if the directory does not exist, it gets
64	case all parent directories will be created if necessary	64		created (and all its parent directories).
65	and the last fraction of the path will be used as the	65		:param str filename name of downloaded file, if not provided basename is
66	file name.	66		extracted from URL
67	:param bool overwrite: if the destination file already exists, whether to	67		:param bool overwrite: if the destination file already exists, whether to
68	overwrite or not. If ``False``, a simple check is	68		overwrite or not. If ``False``, a simple check is
69	performed whether the remote file is the same as the	69		performed whether the remote file is the same as the
70	local file and re-downloads it anyway if they are not	70		local file and re-downloads it anyway if they are not
71	the same.	71		the same.
72	:param grabber: custom :class:`urlgrabber.grabber.URLGrabber` instance to be	72		:param grabber: custom :class:`urlgrabber.grabber.URLGrabber` instance to be
73	used for actual downloading	73		used for actual downloading
74	:return: file path of the downloaded file	74		:return: file path of the downloaded file
75	:rtype: str	75		:rtype: str
76	:raise TaskotronRemoteError: if download fails or directory can't be created	76		:raise TaskotronRemoteError: if download fails or directory can't be created
77	'''	77		'''
78
79	if not grabber:	78		if not grabber:
80	grabber = get_urlgrabber()	79		grabber = get_urlgrabber()
81		80
82	# get destination file, create dirs if needed	81		if not filename:
83	if os.path.isdir(dest):	82		filename = os.path.basename(url)
84	dest = os.path.join(dest, os.path.basename(url))	83
		84
		85		if os.path.exists(dirname):
		86		if not os.path.isdir(dirname):
		87		raise TaskotronRemoteError(
		88		"Can't create directory: %r It is an already "
		89		"existing file.", dirname)
85	else:	90		else:
86	try:	91		try:
87	makedirs(os.path.dirname(dest))	92		makedirs(dirname)
88	except OSError, e:	93		except OSError, e:
89	log.exception("Can't create directory: %s", dest)	94		log.exception("Can't create directory: %s", dest)
90	raise TaskotronRemoteError(e)	95		raise TaskotronRemoteError(e)
91		96
		97		dest = os.path.join(dirname, filename)
		98
92	# overwrite?	99		# overwrite?
93	if os.path.exists(dest):	100		if os.path.exists(dest):
94	if overwrite:	101		if overwrite:
95	os.remove(dest)	102		os.remove(dest)
96	else:	103		else:
97	log.debug('Already downloaded: %s', os.path.basename(dest))	104		log.debug('Already downloaded: %s', os.path.basename(dest))
98	return dest	105		return dest
99		106
100	log.debug('Downloading: %s', url)	107		log.debug('Downloading: %s', url)
101	try:	108		try:
102	grabber.urlgrab(url, dest)	109		grabber.urlgrab(url, dest)
103	except urlgrabber.grabber.URLGrabError, e:	110		except urlgrabber.grabber.URLGrabError, e:
104	log.exception('Download failed for: %s', url)	111		log.exception('Download failed for: %s', url)
105	# the file can be incomplete, remove	112		# the file can be incomplete, remove
106	if os.path.exists(dest):	113		if os.path.exists(dest):
107	try:	114		try:
108	os.remove(dest)	115		os.remove(dest)
109	except OSError, e:	116		except OSError, e:
110	log.exception('Could not delete incomplete file: %s', dest)	117		log.exception('Could not delete incomplete file: %s', dest)
111	raise TaskotronRemoteError(e)	118		raise TaskotronRemoteError(e)
112		119
113	return dest	120		return dest
114		121		No newline at end of file