Ticket #81 (closed defect: fixed)

Opened 21 months ago

Last modified 21 months ago

Incorrect and unclear error message when URL fail to download

Reported by: misc Owned by: misc
Priority: minor Milestone: 0.3.0
Component: fedora-review Version: devel
Keywords: Cc:
Blocked By: Blocking:

Description

As seen on https://bugzilla.redhat.com/show_bug.cgi?id=839064, Fedora review fail to give a useful error message if it cannot download the source froma url :

{{{839064 Bugzilla v0.7.0 initializing Chose subclass RHBugzilla v0.1 Trying bugzilla cookies for authentication Getting .spec and .srpm Urls from : 839064

--> SRPM url: http://brenton.fedorapeople.org/package_reviews/rubygem-stickshift-common/201207201705/rubygem-stickshift-common-0.13.1-3.fc18.src.rpm --> Spec url: http://brenton.fedorapeople.org/package_reviews/rubygem-stickshift-common/201207201705/rubygem-stickshift-common.spec

Using review directory: /home/misc/checkout/git/FedoraReview/839064-rubygem-stickshift-common Downloading .spec and .srpm files No upstream for (Source2): stickshift.if No upstream for (Source3): stickshift.te Downloading (Source0): http://mirror.openshift.com/pub/crankcase/source/stickshift-common-0.13.1.gem Cannot download url: http://mirror.openshift.com/pub/crankcase/source/stickshift-common-0.13.1.gem No upstream for (Source1): stickshift.fc Running checks and generate report

Rebuilding /home/misc/checkout/git/FedoraReview/839064-rubygem-stickshift-common/srpm/rubygem-stickshift-common-0.13.1-3.fc18.src.rpm using default root INFO: Results and/or logs in: /home/misc/checkout/git/FedoraReview/839064-rubygem-stickshift-common/results Build completed Exception down the road... }}}

The cannot download is not clearly display as blocking, and "exception down the road" is too generic ( I think a backtrace is better, since the tool is for a advanced user ).

Anyway, running with -v show this :

Traceback (most recent call last):
  File "/home/misc/checkout/git/FedoraReview/src/FedoraReview/review_helper.py", line 133, in run
    self.__do_report()
  File "/home/misc/checkout/git/FedoraReview/src/FedoraReview/review_helper.py", line 79, in __do_report
    self.__run_checks(self.bug.spec_file, self.bug.srpm_file)
  File "/home/misc/checkout/git/FedoraReview/src/FedoraReview/review_helper.py", line 105, in __run_checks
    writedown=not Settings.no_report)
  File "/home/misc/checkout/git/FedoraReview/src/FedoraReview/checks_class.py", line 180, in run_checks
    if test.is_applicable():
  File "/home/misc/checkout/git/FedoraReview/src/FedoraReview/checks/ccpp.py", line 13, in is_applicable
    self.sources_have_files('*.c') or \
  File "/home/misc/checkout/git/FedoraReview/src/FedoraReview/check_base.py", line 105, in sources_have_files
    sources_files = self.sources.get_files_sources()
  File "/home/misc/checkout/git/FedoraReview/src/FedoraReview/sources.py", line 86, in get_files_sources
    self.extract_all()
  File "/home/misc/checkout/git/FedoraReview/src/FedoraReview/sources.py", line 65, in extract_all
    source.extract()
  File "/home/misc/checkout/git/FedoraReview/src/FedoraReview/source.py", line 105, in extract
    if not self.rpmdev_extract(self.filename, self.extract_dir):
AttributeError: 'Source' object has no attribute 'filename'

Looking at the code, this is because Source.init try to download the file :

            try:
                self.filename = self._get_file(url,
                                               ReviewDirs.upstream,
                                               my_logger)
            except:
                self.log.debug('Download error on ' + url,
                                exc_info=True)
                self.log.warning('Cannot download url: ' + url)
                self.downloaded = False

but the except clause do not set self.filename to a sensible value.

I would propose to fallback to the same system than for local file.

Change History

comment:1 Changed 21 months ago by misc

Here is a patch against git master that fix the issue. I know I could apply myself, but at 3 o clock in the night, I do not trust myself enough to do that :)

comment:2 Changed 21 months ago by misc

  • Owner changed from somebody to misc
  • Status changed from new to assigned

comment:3 Changed 21 months ago by misc

  • Resolution set to fixed
  • Status changed from assigned to closed

Applied the patch on devel, and so close as fixed

Note: See TracTickets for help on using tickets.