openlp-core team mailing list archive

[Raoul Snyman <raoul@xxxxxxxxxxx>]

As per my description ^^, this is a work in progress.

Diff comments:

> 
> === modified file 'openlp/core/common/httputils.py'
> --- openlp/core/common/httputils.py	2017-08-13 05:50:44 +0000
> +++ openlp/core/common/httputils.py	2017-09-20 17:03:28 +0000
> @@ -217,56 +150,42 @@
>      block_count = 0
>      block_size = 4096
>      retries = 0
> -    log.debug("url_get_file: " + url)
> -    while True:
> +    log.debug('url_get_file: %s', url)
> +    while retries < CONNECTION_RETRIES:
>          try:
> -            filename = open(f_path, "wb")
> -            url_file = urllib.request.urlopen(url, timeout=CONNECTION_TIMEOUT)
> -            if sha256:
> -                hasher = hashlib.sha256()
> -            # Download until finished or canceled.
> -            while not callback.was_cancelled:
> -                data = url_file.read(block_size)
> -                if not data:
> -                    break
> -                filename.write(data)
> +            with open(file_path, 'wb') as saved_file:
> +                response = requests.get(url, timeout=float(CONNECTION_TIMEOUT), stream=True)
>                  if sha256:
> -                    hasher.update(data)
> -                block_count += 1
> -                callback._download_progress(block_count, block_size)
> -            filename.close()
> +                    hasher = hashlib.sha256()
> +                # Download until finished or canceled.
> +                for chunk in response.iter_content(chunk_size=block_size):
> +                    if callback.was_cancelled:
> +                        break
> +                    saved_file.write(chunk)
> +                    if sha256:
> +                        hasher.update(chunk)
> +                    block_count += 1
> +                    callback._download_progress(block_count, block_size)
> +                response.close()
>              if sha256 and hasher.hexdigest() != sha256:
> -                log.error('sha256 sums did not match for file: {file}'.format(file=f_path))
> -                os.remove(f_path)
> +                log.error('sha256 sums did not match for file %s, got %s, expected %s', file_path, hasher.hexdigest(),

Yeah. We should be using old style for log messages, at least until we have figured out how to make log use {}. The reason is that the interpolation will only happen when the line is actually logged to file, as opposed to when the code itself is run (i.e. always).

> +                          sha256)
> +                os.remove(file_path)
>                  return False
> -        except (urllib.error.URLError, socket.timeout) as err:
> +            break
> +        except IOError:
>              trace_error_handler(log)
> -            filename.close()
> -            os.remove(f_path)
> +            os.remove(file_path)
>              if retries > CONNECTION_RETRIES:
>                  return False
>              else:
>                  retries += 1
>                  time.sleep(0.1)
>                  continue
> -        break
>      # Delete file if cancelled, it may be a partial file.
>      if callback.was_cancelled:
> -        os.remove(f_path)
> +        os.remove(file_path)
>      return True
>  
>  
> -def ping(host):
> -    """
> -    Returns True if host responds to a ping request
> -    """
> -    # Ping parameters as function of OS
> -    ping_str = "-n 1" if platform.system().lower() == "windows" else "-c 1"
> -    args = "ping " + " " + ping_str + " " + host
> -    need_sh = False if platform.system().lower() == "windows" else True
> -
> -    # Ping
> -    return subprocess.call(args, shell=need_sh) == 0
> -
> -
>  __all__ = ['get_web_page']
> 
> === renamed file 'openlp/core/common/versionchecker.py' => 'openlp/core/version.py'
> --- openlp/core/common/versionchecker.py	2017-08-03 17:54:40 +0000
> +++ openlp/core/version.py	2017-09-20 17:03:28 +0000
> @@ -46,42 +44,93 @@
>  CONNECTION_RETRIES = 2
>  
>  
> -class VersionThread(QtCore.QThread):
> -    """
> -    A special Qt thread class to fetch the version of OpenLP from the website.
> -    This is threaded so that it doesn't affect the loading time of OpenLP.
> -    """
> -    def __init__(self, main_window):
> -        """
> -        Constructor for the thread class.
> -
> -        :param main_window: The main window Object.
> -        """
> -        log.debug("VersionThread - Initialise")
> -        super(VersionThread, self).__init__(None)
> -        self.main_window = main_window
> -
> -    def run(self):
> -        """
> -        Run the thread.
> -        """
> -        self.sleep(1)
> -        log.debug('Version thread - run')
> -        found = ping("openlp.io")
> -        Registry().set_flag('internet_present', found)
> -        update_check = Settings().value('core/update check')
> -        if found:
> -            Registry().execute('get_website_version')
> -            if update_check:
> -                app_version = get_application_version()
> -                version = check_latest_version(app_version)
> -                log.debug("Versions {version1} and {version2} ".format(version1=LooseVersion(str(version)),
> -                                                                       version2=LooseVersion(str(app_version['full']))))
> -                if LooseVersion(str(version)) > LooseVersion(str(app_version['full'])):
> -                    self.main_window.openlp_version_check.emit('{version}'.format(version=version))
> -
> -
> -def get_application_version():
> +class VersionWorker(QtCore.QObject):
> +    """
> +    A worker class to fetch the version of OpenLP from the website. This is run from within a thread so that it
> +    doesn't affect the loading time of OpenLP.
> +    """
> +    new_version = QtCore.pyqtSignal(dict)
> +    no_internet = QtCore.pyqtSignal()
> +    quit = QtCore.pyqtSignal()
> +
> +    def __init__(self, last_check_date, current_version):
> +        """
> +        Constructor for the version check worker.
> +
> +        :param string last_check_date: The last day we checked for a new version of OpenLP
> +        """
> +        log.debug('VersionWorker - Initialise')
> +        super(VersionWorker, self).__init__(None)
> +        self.last_check_date = last_check_date
> +        self.current_version = current_version
> +
> +    def start(self):
> +        """
> +        Check the latest version of OpenLP against the version file on the OpenLP site.
> +
> +        **Rules around versions and version files:**
> +
> +        * If a version number has a build (i.e. -bzr1234), then it is a nightly.
> +        * If a version number's minor version is an odd number, it is a development release.
> +        * If a version number's minor version is an even number, it is a stable release.
> +        """
> +        log.debug('VersionWorker - Start')
> +        # I'm not entirely sure why this was here, I'm commenting it out until I hit the same scenario

Yes. It was commented out at one stage. Then I had some issues, so I put it back in to see if it would help. It didn't. I need to take it out again and see what it does.

> +        time.sleep(1)
> +        download_url = 'http://www.openlp.org/files/version.txt'
> +        if self.current_version['build']:
> +            download_url = 'http://www.openlp.org/files/nightly_version.txt'
> +        elif int(self.current_version['version'].split('.')[1]) % 2 != 0:
> +            download_url = 'http://www.openlp.org/files/dev_version.txt'
> +        headers = {
> +            'User-Agent': 'OpenLP/{version} {system}/{release}; '.format(version=self.current_version['full'],
> +                                                                         system=platform.system(),
> +                                                                         release=platform.release())
> +        }
> +        remote_version = None
> +        retries = 0
> +        while retries < 3:
> +            try:
> +                response = requests.get(download_url, headers=headers)
> +                remote_version = response.text
> +                log.debug('New version found: %s', remote_version)
> +                break
> +            except IOError:
> +                log.exception('Unable to connect to OpenLP server to download version file')
> +                retries += 1
> +        else:
> +            self.no_internet.emit()
> +        if remote_version and LooseVersion(remote_version) > LooseVersion(self.current_version['full']):
> +            self.new_version.emit(remote_version)
> +        self.quit.emit()
> +
> +
> +def update_check_date():
> +    """
> +    Save when we last checked for an update
> +    """
> +    Settings().setValue('core/last version test', date.today().strftime('%Y-%m-%d'))
> +
> +
> +def check_for_update(parent):
> +    """
> +    Run a thread to download and check the version of OpenLP
> +
> +    :param MainWindow parent: The parent object for the thread. Usually the OpenLP main window.
> +    """
> +    last_check_date = Settings().value('core/last version test')
> +    if date.today().strftime('%Y-%m-%d') <= last_check_date:
> +        log.debug('Version check skipped, last checked today')
> +        return
> +    worker = VersionWorker(last_check_date, get_version())
> +    worker.new_version.connect(parent.on_new_version)
> +    worker.quit.connect(update_check_date)
> +    # TODO: Use this to figure out if there's an Internet connection?
> +    # worker.no_internet.connect(parent.on_no_internet)
> +    run_thread(parent, worker, 'version')
> +
> +
> +def get_version():
>      """
>      Returns the application version of the running instance of OpenLP::
>  


-- 
https://code.launchpad.net/~raoul-snyman/openlp/fix-version-check/+merge/331081
Your team OpenLP Core is subscribed to branch lp:openlp.