Replaced URLGrabber with requests (rhbz#1141245)

Because urlgrabber is being dropped by many significant software projects in
Fedora, Pykickstart can't keep depending on it once it is converted to Python 3
(rhbz#985310).

This commit replaces the dep with requests with SSL verification turned on by
default. This could pose trouble in Python 2 if proper underlying packages
aren't installed [1]. In case this becomes a problem, the SSL_VERIFY constant
in load.py may be switched to False.

[1] https://github.com/kennethreitz/requests/issues/749#issuecomment-19187417

pykickstart.spec

@@ -15,11 +15,11 @@ Group: System Environment/Libraries
 BuildArch: noarch
 BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 BuildRequires: python-devel, gettext, python-setuptools
-BuildRequires: python-urlgrabber
+BuildRequires: python-requests
 %if ! 0%{?rhel}
 BuildRequires: transifex-client
 %endif
-Requires: python, python-urlgrabber
+Requires: python, python-requests
 
 %description
 The pykickstart package is a python library for manipulating kickstart

pykickstart/load.py

@@ -0,0 +1,107 @@
+#
+# Tomas Radej <[email protected]>
+#
+# Copyright 2015 Red Hat, Inc.
+#
+# This copyrighted material is made available to anyone wishing to use, modify,
+# copy, or redistribute it subject to the terms and conditions of the GNU
+# General Public License v.2.  This program is distributed in the hope that it
+# will be useful, but WITHOUT ANY WARRANTY expressed or implied, including the
+# implied warranties of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+# See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program; if not, write to the Free Software Foundation, Inc., 51
+# Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  Any Red Hat
+# trademarks that are incorporated in the source code or documentation are not
+# subject to the GNU General Public License and may only be used or replicated
+# with the express permission of Red Hat, Inc.
+#
+import requests
+import shutil
+
+from pykickstart.errors import KickstartError
+from requests.exceptions import SSLError, RequestException
+
+import gettext
+_ = lambda x: gettext.ldgettext("pykickstart", x)
+
+_is_url = lambda location: '://' in location # RFC 3986
+
+SSL_VERIFY = True
+
+def load_to_str(location):
+    '''Load a destination URL or file into a string.
+    Type of input is inferred automatically.
+
+    Arguments:
+    location -- URL or file name to load
+
+    Returns: string with contents
+    Raises: KickstartError on error reading'''
+
+    if _is_url(location):
+        return _load_url(location)
+    else:
+        return _load_file(location)
+
+
+def load_to_file(location, destination):
+    '''Load a destination URL or file into a file name.
+    Type of input is inferred automatically.
+
+    Arguments:
+    location -- URL or file name to load
+    destination -- destination file name to write to
+
+    Returns: file name with contents
+    Raises: KickstartError on error reading or writing'''
+
+    if _is_url(location):
+        contents =  _load_url(location)
+
+        # Write to file
+        try:
+            with open(destination, 'w') as fh:
+                fh.write(contents)
+        except IOError as e:
+            raise KickstartError(_('Error writing file "%s":') % location + ': {e}'.format(e=str(e)))
+
+        return destination
+    else:
+        _copy_file(location, destination)
+        return destination
+
+
+
+def _load_url(location):
+    '''Load a location (URL or filename) and return contents as string'''
+
+    try:
+        request = requests.get(location, verify=SSL_VERIFY)
+    except SSLError as e:
+        raise KickstartError(_('Error securely accessing URL "%s"') % location + ': {e}'.format(e=str(e)))
+    except RequestException as e:
+        raise KickstartError(_('Error accessing URL "%s"') % location + ': {e}'.format(e=str(e)))
+
+    return request.content
+
+
+def _load_file(filename):
+    '''Load a file's contents and return them as a string'''
+
+    try:
+        with open(filename, 'r') as fh:
+            contents = fh.read()
+    except IOError as e:
+        raise KickstartError(_('Error opening file: %s') % str(e))
+
+    return contents
+
+def _copy_file(filename, destination):
+    '''Copy file to destination'''
+
+    try:
+        shutil.copyfile(filename, destination)
+    except OSError as e:
+        raise KickstartError(_('Error copying file: %s') % str(e))

pykickstart/parser.py

@@ -36,12 +36,11 @@
 import shlex
 import tempfile
 from optparse import OptionParser
-from urlgrabber import urlread
-import urlgrabber.grabber as grabber
 
 from pykickstart import constants, version
 from pykickstart.errors import KickstartError, KickstartParseError, KickstartValueError, formatErrorMsg
 from pykickstart.ko import KickstartObject
+from pykickstart.load import load_to_str
 from pykickstart.sections import PackageSection, PreScriptSection, PostScriptSection, TracebackScriptSection
 
 import gettext
@@ -85,25 +84,16 @@ def _preprocessStateMachine (lineIter):
             raise KickstartParseError(formatErrorMsg(lineno, msg=_("Illegal url for %%ksappend: %s") % ll))
 
         try:
-            url = grabber.urlopen(ksurl)
-        except grabber.URLGrabError, e:
-            raise KickstartError(formatErrorMsg(lineno, msg=_("Unable to open %%ksappend file: %s") % e.strerror))
-        else:
-            # Sanity check result.  Sometimes FTP doesn't catch a file
-            # is missing.
-            try:
-                if url.size < 1:
-                    raise KickstartError(formatErrorMsg(lineno, msg=_("Unable to open %%ksappend file")))
-            except:
-                raise KickstartError(formatErrorMsg(lineno, msg=_("Unable to open %%ksappend file")))
+            contents = load_to_str(ksurl)
+        except KickstartError as e:
+            raise KickstartError(formatErrorMsg(lineno, msg=_("Unable to open %%ksappend file: %s") % str(e)))
 
         # If that worked, write the remote file to the output kickstart
         # file in one burst.  Then close everything up to get ready to
         # read ahead in the input file.  This allows multiple %ksappend
         # lines to exist.
-        if url is not None:
-            os.write(outF, url.read())
-            url.close()
+        if contents is not None:
+            os.write(outF, contents)
 
     # All done - close the temp file and return its location.
     os.close(outF)
@@ -126,13 +116,11 @@ def preprocessKickstart (f):
         run.  Returns the location of the complete kickstart file.
     """
     try:
-        fh = grabber.urlopen(f)
-    except grabber.URLGrabError, e:
-        raise KickstartError(formatErrorMsg(0, msg=_("Unable to open input kickstart file: %s") % e.strerror))
+        contents = load_to_str(f)
+    except KickstartError as e:
+        raise KickstartError(formatErrorMsg(0, msg=_("Unable to open input kickstart file: %s") % str(e)))
 
-    rc = _preprocessStateMachine (iter(fh.readlines()))
-    fh.close()
-    return rc
+    return _preprocessStateMachine (iter(contents.splitlines(True)))
 
 class PutBackIterator(Iterator):
     def __init__(self, iterable):
@@ -726,9 +714,9 @@ def readKickstart(self, f, reset=True):
         self.currentdir[self._includeDepth] = cd
 
         try:
-            s = urlread(f)
-        except grabber.URLGrabError, e:
-            raise KickstartError(formatErrorMsg(0, msg=_("Unable to open input kickstart file: %s") % e.strerror))
+            s = load_to_str(f)
+        except KickstartError as e:
+            raise KickstartError(formatErrorMsg(0, msg=_("Unable to open input kickstart file: %s") % str(e)))
 
         self.readKickstartFromString(s, reset=False)
 

pykickstart/version.py

@@ -44,12 +44,12 @@
                       have a version= comment in it.
 """
 import imputil, re, sys
-from urlgrabber import urlopen
 
 import gettext
 _ = lambda x: gettext.ldgettext("pykickstart", x)
 
 from pykickstart.errors import KickstartVersionError
+from pykickstart.load import load_to_str
 
 # Symbolic names for internal version numbers.
 RHEL3 = 900
@@ -147,26 +147,16 @@ def versionFromFile(f):
     """
     v = DEVEL
 
-    fh = urlopen(f)
+    contents = load_to_str(f)
 
-    while True:
-        try:
-            l = fh.readline()
-        except StopIteration:
-            break
-
-        # At the end of the file?
-        if l == "":
-            break
-
-        if l.isspace() or l.strip() == "":
+    for line in contents.splitlines(True):
+        if line.strip() == "":
             continue
 
-        if l[:9] == "#version=":
-            v = stringToVersion(l[9:].rstrip())
+        if line[:9] == "#version=":
+            v = stringToVersion(line[9:].rstrip())
             break
 
-    fh.close()
     return v
 
 def returnClassForVersion(version=DEVEL):

tools/ksvalidator

@@ -24,9 +24,9 @@ import os
 import sys
 import warnings
 import tempfile
-import urlgrabber
 import shutil
 from pykickstart.errors import KickstartError, KickstartParseError, KickstartValueError, KickstartVersionError
+from pykickstart.load import load_to_file
 from pykickstart.parser import KickstartParser, preprocessKickstart
 from pykickstart.version import DEVEL, makeVersion, versionMap
 
@@ -70,8 +70,8 @@ if opts.listversions:
 
 destdir = tempfile.mkdtemp("", "ksvalidator-tmp-", "/tmp")
 try:
-    f = urlgrabber.urlgrab(opts.ksfile, filename="%s/ks.cfg" % destdir)
-except urlgrabber.grabber.URLGrabError, e:
+    f = load_to_file(opts.ksfile, "%s/ks.cfg" % destdir)
+except KickstartError as e:
     print(_("Error reading %s:\n%s") % (opts.ksfile, e))
     cleanup(destdir)