diff --git a/libtaskotron/file_utils.py b/libtaskotron/file_utils.py --- a/libtaskotron/file_utils.py +++ b/libtaskotron/file_utils.py @@ -56,14 +56,14 @@ return grabber -def download(url, dest, overwrite=False, grabber=None): +def download(url, dirname, filename=None, overwrite=False, grabber=None): '''Download a file. :param str url: file URL to download - :param str dest: either an existing directory or a file path. In the latter - case all parent directories will be created if necessary - and the last fraction of the path will be used as the - file name. + :param str dirname directory path, if the directory does not exist, it gets + created (and all its parent directories). + :param str filename name of downloaded file, if not provided basename is + extracted from URL :param bool overwrite: if the destination file already exists, whether to overwrite or not. If ``False``, a simple check is performed whether the remote file is the same as the @@ -75,20 +75,27 @@ :rtype: str :raise TaskotronRemoteError: if download fails or directory can't be created ''' - if not grabber: grabber = get_urlgrabber() - # get destination file, create dirs if needed - if os.path.isdir(dest): - dest = os.path.join(dest, os.path.basename(url)) + if not filename: + filename = os.path.basename(url) + + + if os.path.exists(dirname): + if not os.path.isdir(dirname): + raise TaskotronRemoteError( + "Can't create directory: %r It is an already " + "existing file.", dirname) else: try: - makedirs(os.path.dirname(dest)) + makedirs(dirname) except OSError, e: log.exception("Can't create directory: %s", dest) raise TaskotronRemoteError(e) + dest = os.path.join(dirname, filename) + # overwrite? if os.path.exists(dest): if overwrite: @@ -110,5 +117,4 @@ log.exception('Could not delete incomplete file: %s', dest) raise TaskotronRemoteError(e) - return dest - + return dest \ No newline at end of file diff --git a/libtaskotron/koji_utils.py b/libtaskotron/koji_utils.py --- a/libtaskotron/koji_utils.py +++ b/libtaskotron/koji_utils.py @@ -123,33 +123,6 @@ :raise TaskotronRemoteError: if issues with target directory ''' - # Create the rpm_dir if if does not exist - # as fileutils.download requires existing directory - # If this is not present, then the first rpm to be downloaded - # will fail to download, because of how file_utils.download() - # is currently written. On line #79, the `dest` is not a dir - # (because it is not yet created), so it is created on line #83 - # but as an effect, `dest` is still containing just the dirname - # instead of dirname+rpmname (because line #80 was not executed - # so the lines #89-94 end up going through the else branch - # thus not downloading the first file. - # All subsequent downloads are OK, since the rpm_dir (aka dest) - # already exists. - # YAY for two hours of debugging! - # FIXME: this should be solved better, somehow... - if os.path.exists(rpm_dir): - if not os.path.isdir(rpm_dir): - raise exc.TaskotronRemoteError( - "Can't create directory: %r It is an already " - "existing file.", rpm_dir) - else: - try: - file_utils.makedirs(rpm_dir) - except OSError, e: - log.exception("Can't create directory: %s", rpm_dir) - raise exc.TaskotronRemoteError(e) - - try: rpm_urls = self.nvr_to_urls(nvr, arches=arches,