Browse Source

[filebrowser] Fix filebroser to be compatible with webhdfs

* Unit tests run against webhdfs
* Decode/render i18n paths correctly
* Fixed upload handling
bc Wong 13 years ago
parent
commit
bb127254ba

+ 2 - 1
apps/filebrowser/src/filebrowser/templates/listdir_components.mako

@@ -18,6 +18,7 @@ import datetime
 import hashlib
 from django.template.defaultfilters import urlencode, stringformat, filesizeformat, date, time, escape
 from desktop.lib.django_util import reverse_with_get
+from django.utils.encoding import smart_str
 %>
 
 
@@ -123,7 +124,7 @@ from desktop.lib.django_util import reverse_with_get
              % if ".." != file['name']:
 				<%
 				m = hashlib.md5()
-				m.update(path)
+				m.update(smart_str(path))
 				%>
 				<a class="btn small contextEnabler" data-menuid="${urlencode(m.hexdigest())}">Options</a>
 				<ul class="contextMenu" id="menu${urlencode(m.hexdigest())}">

+ 1 - 1
apps/filebrowser/src/filebrowser/views.py

@@ -238,7 +238,7 @@ def _do_overwrite_save(fs, path, data, encoding):
     # Try to match the permissions and ownership of the old file
     cur_stats = fs.stats(path)
     try:
-        fs.chmod(path_dest, cur_stats['mode'])
+        fs.chmod(path_dest, stat_module.S_IMODE(cur_stats['mode']))
     except:
         logging.warn("Could not chmod new file %s to match old file %s" % (
             path_dest, path), exc_info=True)

+ 33 - 39
apps/filebrowser/src/filebrowser/views_test.py

@@ -18,8 +18,9 @@
 """
 Tests for filebrowser views
 """
+from django.utils.encoding import smart_str
 from nose.plugins.attrib import attr
-from hadoop import mini_cluster
+from hadoop import pseudo_hdfs4
 from avro import schema, datafile, io
 from desktop.lib.django_test_util import make_logged_in_client
 from nose.tools import assert_true, assert_false, assert_equal
@@ -29,36 +30,35 @@ LOG = logging.getLogger(__name__)
 
 @attr('requires_hadoop')
 def test_chown():
-  cluster = mini_cluster.shared_cluster(conf=True)
-  try:
-    # Only the Hadoop superuser really has carte blanche here
-    c = make_logged_in_client(cluster.superuser)
-    cluster.fs.setuser(cluster.superuser)
+  cluster = pseudo_hdfs4.shared_cluster()
+
+  # Only the Hadoop superuser really has carte blanche here
+  c = make_logged_in_client(cluster.superuser)
+  cluster.fs.setuser(cluster.superuser)
+
+  PATH = u"/test-chown-en-Español"
+  cluster.fs.mkdir(PATH)
+  c.post("/filebrowser/chown", dict(path=PATH, user="x", group="y"))
+  assert_equal("x", cluster.fs.stats(PATH)["user"])
+  assert_equal("y", cluster.fs.stats(PATH)["group"])
+  c.post("/filebrowser/chown", dict(path=PATH, user="__other__", user_other="z", group="y"))
+  assert_equal("z", cluster.fs.stats(PATH)["user"])
+
+  # Make sure that the regular user chown form doesn't have useless fields,
+  # and that the superuser's form has all the fields it could dream of.
+  PATH = '/filebrowser/chown-regular-user'
+  cluster.fs.mkdir(PATH)
+  cluster.fs.chown(PATH, 'chown_test', 'chown_test')
+  response = c.get('/filebrowser/chown', dict(path=PATH, user='chown_test', group='chown_test'))
+  assert_true('<option value="__other__"' in response.content)
+  c = make_logged_in_client('chown_test')
+  response = c.get('/filebrowser/chown', dict(path=PATH, user='chown_test', group='chown_test'))
+  assert_false('<option value="__other__"' in response.content)
 
-    PATH = u"/test-chown-en-Español"
-    cluster.fs.mkdir(PATH)
-    c.post("/filebrowser/chown", dict(path=PATH, user="x", group="y"))
-    assert_equal("x", cluster.fs.stats(PATH)["user"])
-    assert_equal("y", cluster.fs.stats(PATH)["group"])
-    c.post("/filebrowser/chown", dict(path=PATH, user="__other__", user_other="z", group="y"))
-    assert_equal("z", cluster.fs.stats(PATH)["user"])
-
-    # Make sure that the regular user chown form doesn't have useless fields,
-    # and that the superuser's form has all the fields it could dream of.
-    PATH = '/filebrowser/chown-regular-user'
-    cluster.fs.mkdir(PATH)
-    cluster.fs.chown(PATH, 'chown_test', 'chown_test')
-    response = c.get('/filebrowser/chown', dict(path=PATH, user='chown_test', group='chown_test'))
-    assert_true('<option value="__other__"' in response.content)
-    c = make_logged_in_client('chown_test')
-    response = c.get('/filebrowser/chown', dict(path=PATH, user='chown_test', group='chown_test'))
-    assert_false('<option value="__other__"' in response.content)
-  finally:
-    cluster.shutdown()
 
 @attr('requires_hadoop')
 def test_listdir():
-  cluster = mini_cluster.shared_cluster(conf=True)
+  cluster = pseudo_hdfs4.shared_cluster()
   try:
     c = make_logged_in_client()
     cluster.fs.setuser(cluster.superuser)
@@ -104,11 +104,10 @@ def test_listdir():
       cluster.fs.rmtree('/user/test')
     except:
       pass      # Don't let cleanup errors mask earlier failures
-    cluster.shutdown()
 
 @attr('requires_hadoop')
 def test_view_avro():
-  cluster = mini_cluster.shared_cluster(conf=True)
+  cluster = pseudo_hdfs4.shared_cluster()
   try:
     c = make_logged_in_client()
     cluster.fs.setuser(cluster.superuser)
@@ -166,11 +165,10 @@ def test_view_avro():
       cluster.fs.rmtree('/test-avro-filebrowser/')
     except:
       pass      # Don't let cleanup errors mask earlier failures
-    cluster.shutdown()
 
 @attr('requires_hadoop')
 def test_view_gz():
-  cluster = mini_cluster.shared_cluster(conf=True)
+  cluster = pseudo_hdfs4.shared_cluster()
   try:
     c = make_logged_in_client()
     cluster.fs.setuser(cluster.superuser)
@@ -212,12 +210,11 @@ def test_view_gz():
       cluster.fs.rmtree('/test-gz-filebrowser/')
     except:
       pass      # Don't let cleanup errors mask earlier failures
-    cluster.shutdown()
 
 
 @attr('requires_hadoop')
 def test_view_i18n():
-  cluster = mini_cluster.shared_cluster(conf=True)
+  cluster = pseudo_hdfs4.shared_cluster()
   try:
     cluster.fs.setuser(cluster.superuser)
     cluster.fs.mkdir('/test-filebrowser/')
@@ -250,7 +247,6 @@ def test_view_i18n():
       cluster.fs.rmtree('/user/test')
     except Exception, ex:
       LOG.error('Failed to cleanup test directory: %s' % (ex,))
-    cluster.shutdown()
 
 
 def view_helper(cluster, encoding, content):
@@ -282,7 +278,7 @@ def view_helper(cluster, encoding, content):
 
 @attr('requires_hadoop')
 def test_edit_i18n():
-  cluster = mini_cluster.shared_cluster(conf=True)
+  cluster = pseudo_hdfs4.shared_cluster()
   try:
     cluster.fs.setuser(cluster.superuser)
     cluster.fs.mkdir('/test-filebrowser/')
@@ -312,7 +308,6 @@ def test_edit_i18n():
       cluster.fs.rmtree('/test-filebrowser/')
     except Exception, ex:
       LOG.error('Failed to remove tree /test-filebrowser: %s' % (ex,))
-    cluster.shutdown()
 
 
 def edit_helper(cluster, encoding, contents_pass_1, contents_pass_2):
@@ -366,13 +361,13 @@ def edit_helper(cluster, encoding, contents_pass_1, contents_pass_2):
     try:
       cluster.fs.remove(filename)
     except Exception, ex:
-      LOG.error('Failed to remove %s: %s' % (filename, ex))
+      LOG.error('Failed to remove %s: %s' % (smart_str(filename), ex))
 
 
 @attr('requires_hadoop')
 def test_upload():
   """Test file upload"""
-  cluster = mini_cluster.shared_cluster(conf=True)
+  cluster = pseudo_hdfs4.shared_cluster()
   try:
     USER_NAME = cluster.fs.superuser
     cluster.fs.setuser(USER_NAME)
@@ -397,4 +392,3 @@ def test_upload():
       cluster.fs.remove(DEST)
     except Exception, ex:
       pass
-    cluster.shutdown()

+ 63 - 1
desktop/core/src/desktop/lib/rest/http_client.py

@@ -17,6 +17,7 @@
 import cookielib
 import logging
 import posixpath
+import types
 import urllib
 import urllib2
 
@@ -155,4 +156,65 @@ class HttpClient(object):
     if params:
       param_str = urllib.urlencode(params)
       res += '?' + param_str
-    return res
+    return iri_to_uri(res)
+
+
+#
+# Method copied from Django
+#
+def iri_to_uri(iri):
+    """
+    Convert an Internationalized Resource Identifier (IRI) portion to a URI
+    portion that is suitable for inclusion in a URL.
+
+    This is the algorithm from section 3.1 of RFC 3987.  However, since we are
+    assuming input is either UTF-8 or unicode already, we can simplify things a
+    little from the full method.
+
+    Returns an ASCII string containing the encoded result.
+    """
+    # The list of safe characters here is constructed from the "reserved" and
+    # "unreserved" characters specified in sections 2.2 and 2.3 of RFC 3986:
+    #     reserved    = gen-delims / sub-delims
+    #     gen-delims  = ":" / "/" / "?" / "#" / "[" / "]" / "@"
+    #     sub-delims  = "!" / "$" / "&" / "'" / "(" / ")"
+    #                   / "*" / "+" / "," / ";" / "="
+    #     unreserved  = ALPHA / DIGIT / "-" / "." / "_" / "~"
+    # Of the unreserved characters, urllib.quote already considers all but
+    # the ~ safe.
+    # The % character is also added to the list of safe characters here, as the
+    # end of section 3.1 of RFC 3987 specifically mentions that % must not be
+    # converted.
+    if iri is None:
+        return iri
+    return urllib.quote(smart_str(iri), safe="/#%[]=:;$&()+,!?*@'~")
+
+#
+# Method copied from Django
+#
+def smart_str(s, encoding='utf-8', strings_only=False, errors='strict'):
+    """
+    Returns a bytestring version of 's', encoded as specified in 'encoding'.
+
+    If strings_only is True, don't convert (some) non-string-like objects.
+    """
+    if strings_only and isinstance(s, (types.NoneType, int)):
+        return s
+    elif not isinstance(s, basestring):
+        try:
+            return str(s)
+        except UnicodeEncodeError:
+            if isinstance(s, Exception):
+                # An Exception subclass containing non-ASCII data that doesn't
+                # know how to print itself properly. We shouldn't raise a
+                # further exception.
+                return ' '.join([smart_str(arg, encoding, strings_only,
+                        errors) for arg in s])
+            return unicode(s).encode(encoding, errors)
+    elif isinstance(s, unicode):
+        return s.encode(encoding, errors)
+    elif s and encoding != 'utf-8':
+        return s.decode('utf-8', errors).encode(encoding, errors)
+    else:
+        return s
+

+ 3 - 2
desktop/core/src/desktop/lib/rest/resource.py

@@ -64,8 +64,9 @@ class Resource(object):
         (method, body[:32], len(body) > 32 and "..." or ""))
 
     # Is the response application/json?
-    if resp.info().getmaintype() == "application" and \
-         resp.info().getsubtype() == "json":
+    if len(body) != 0 and \
+          resp.info().getmaintype() == "application" and \
+          resp.info().getsubtype() == "json":
       try:
         json_dict = json.loads(body)
         return json_dict

+ 12 - 3
desktop/libs/hadoop/src/hadoop/fs/upload.py

@@ -41,7 +41,7 @@ LOG = logging.getLogger(__name__)
 class HDFSerror(Exception):
   pass
 
-class HDFStemporaryUploadedFile(hadoop.fs.hadoopfs.FileUpload):
+class HDFStemporaryUploadedFile(object):
   """
   A temporary HDFS file to store upload data.
   This class does not have any file read methods.
@@ -71,7 +71,7 @@ class HDFStemporaryUploadedFile(hadoop.fs.hadoopfs.FileUpload):
 
     # Make the tmp dir 0777
     self._fs.chmod(self._fs.dirname(self._path), 0777)
-    hadoop.fs.hadoopfs.FileUpload.__init__(self, self._fs, self._path)
+    self._file = self._fs.open(self._path, 'w')
     self._do_cleanup = True
 
   def __del__(self):
@@ -88,7 +88,7 @@ class HDFStemporaryUploadedFile(hadoop.fs.hadoopfs.FileUpload):
       self.size = size
       self.close()
     except Exception, ex:
-      LOG.exception('Error uploading file to %s' % (self.path,))
+      LOG.exception('Error uploading file to %s' % (self._path,))
       raise
 
   def remove(self):
@@ -100,6 +100,15 @@ class HDFStemporaryUploadedFile(hadoop.fs.hadoopfs.FileUpload):
         LOG.exception('Failed to remove temporary upload file "%s". '
                       'Please cleanup manually: %s' % (self._path, ex))
 
+  def write(self, data):
+    self._file.write(data)
+
+  def flush(self):
+    self._file.flush()
+
+  def close(self):
+    self._file.close()
+
 
 class HDFSfileUploadHandler(FileUploadHandler):
   """

+ 24 - 15
desktop/libs/hadoop/src/hadoop/fs/webhdfs.py

@@ -27,11 +27,11 @@ import threading
 from django.utils.encoding import smart_str
 from desktop.lib.rest import http_client, resource
 from hadoop.fs import normpath, SEEK_SET, SEEK_CUR, SEEK_END
-from hadoop.fs.hadoopfs import encode_fs_path, Hdfs
+from hadoop.fs.hadoopfs import Hdfs
 from hadoop.fs.exceptions import WebHdfsException
 from hadoop.fs.webhdfs_types import WebHdfsStat, WebHdfsContentSummary
 
-
+DEFAULT_HDFS_SUPERUSER = 'hdfs'
 DEFAULT_USER = 'hue_webui'
 
 # The number of bytes to read if not specified
@@ -44,7 +44,7 @@ class WebHdfs(Hdfs):
   WebHdfs implements the filesystem interface via the WebHDFS rest protocol.
   """
   def __init__(self, url,
-               hdfs_superuser="hdfs",
+               hdfs_superuser=None,
                security_enabled=False,
                temp_dir="/tmp"):
     self._url = url
@@ -81,6 +81,15 @@ class WebHdfs(Hdfs):
 
   @property
   def superuser(self):
+    if self._superuser is None:
+      try:
+        # The owner of '/' is usually the superuser
+        sb = self.stats('/')
+        self._superuser = sb.user
+      except Exception, ex:
+        LOG.exception('Failed to determine superuser of %s: %s' % (self, ex))
+        self._superuser = DEFAULT_HDFS_SUPERUSER
+
     return self._superuser
   
   @property
@@ -100,7 +109,7 @@ class WebHdfs(Hdfs):
 
     Get directory listing with stats.
     """
-    path = encode_fs_path(Hdfs.normpath(path))
+    path = Hdfs.normpath(path)
     params = self._getparams()
     if glob is not None:
       params['filter'] = glob
@@ -122,7 +131,7 @@ class WebHdfs(Hdfs):
     """
     get_content_summary(path) -> WebHdfsContentSummary
     """
-    path = encode_fs_path(Hdfs.normpath(path))
+    path = Hdfs.normpath(path)
     params = self._getparams()
     params['op'] = 'GETCONTENTSUMMARY'
     json = self._root.get(path, params)
@@ -131,7 +140,7 @@ class WebHdfs(Hdfs):
 
   def _stats(self, path):
     """This version of stats returns None if the entry is not found"""
-    path = encode_fs_path(Hdfs.normpath(path))
+    path = Hdfs.normpath(path)
     params = self._getparams()
     params['op'] = 'GETFILESTATUS'
     try:
@@ -172,7 +181,7 @@ class WebHdfs(Hdfs):
 
     Delete a file or directory.
     """
-    path = encode_fs_path(Hdfs.normpath(path))
+    path = Hdfs.normpath(path)
     params = self._getparams()
     params['op'] = 'DELETE'
     params['recursive'] = recursive and 'true' or 'false'
@@ -200,7 +209,7 @@ class WebHdfs(Hdfs):
 
     Creates a directory and any parent directory if necessary.
     """
-    path = encode_fs_path(Hdfs.normpath(path))
+    path = Hdfs.normpath(path)
     params = self._getparams()
     params['op'] = 'MKDIRS'
     if mode is not None:
@@ -228,8 +237,8 @@ class WebHdfs(Hdfs):
 
   def rename(self, old, new):
     """rename(old, new)"""
-    old = encode_fs_path(Hdfs.normpath(old))
-    new = encode_fs_path(Hdfs.normpath(new))
+    old = Hdfs.normpath(old)
+    new = Hdfs.normpath(new)
     params = self._getparams()
     params['op'] = 'RENAME'
     # Encode `new' because it's in the params
@@ -241,7 +250,7 @@ class WebHdfs(Hdfs):
 
   def chown(self, path, user=None, group=None):
     """chown(path, user=None, group=None)"""
-    path = encode_fs_path(Hdfs.normpath(path))
+    path = Hdfs.normpath(path)
     params = self._getparams()
     params['op'] = 'SETOWNER'
     if user is not None:
@@ -252,7 +261,7 @@ class WebHdfs(Hdfs):
 
   def chmod(self, path, mode):
     """chmod(path, mode)"""
-    path = encode_fs_path(Hdfs.normpath(path))
+    path = Hdfs.normpath(path)
     params = self._getparams()
     params['op'] = 'SETPERMISSION'
     params['permission'] = safe_octal(mode)
@@ -272,7 +281,7 @@ class WebHdfs(Hdfs):
 
     Read data from a file.
     """
-    path = encode_fs_path(Hdfs.normpath(path))
+    path = Hdfs.normpath(path)
     params = self._getparams()
     params['op'] = 'OPEN'
     params['offset'] = long(offset)
@@ -299,7 +308,7 @@ class WebHdfs(Hdfs):
 
     Creates a file with the specified parameters.
     """
-    path = encode_fs_path(Hdfs.normpath(path))
+    path = Hdfs.normpath(path)
     params = self._getparams()
     params['op'] = 'CREATE'
     params['overwrite'] = overwrite and 'true' or 'false'
@@ -319,7 +328,7 @@ class WebHdfs(Hdfs):
 
     Append data to a given file.
     """
-    path = encode_fs_path(Hdfs.normpath(path))
+    path = Hdfs.normpath(path)
     params = self._getparams()
     params['op'] = 'APPEND'
     self._invoke_with_redirect('POST', path, params, data)

+ 5 - 4
desktop/libs/hadoop/src/hadoop/fs/webhdfs_types.py

@@ -21,7 +21,8 @@ Return types from WebHDFS api calls.
 
 import stat
 
-from hadoop.fs.hadoopfs import Hdfs
+from django.utils.encoding import smart_str
+from hadoop.fs.hadoopfs import Hdfs, decode_fs_path
 
 class WebHdfsStat(object):
   """
@@ -31,7 +32,7 @@ class WebHdfsStat(object):
   """
 
   def __init__(self, file_status, parent_path):
-    self.path = Hdfs.join(parent_path, file_status['pathSuffix'])
+    self.path = Hdfs.join(parent_path, decode_fs_path(file_status['pathSuffix']))
     self.isDir = file_status['type'] == 'DIRECTORY'
     self.atime = file_status['accessTime'] / 1000
     self.mtime = file_status['modificationTime'] / 1000
@@ -47,13 +48,13 @@ class WebHdfsStat(object):
     else:
       self.mode |= stat.S_IFREG
 
-  def __str__(self):
+  def __unicode__(self):
     return "[WebHdfsStat] %7s %8s %8s %12s %s%s" % \
         (oct(self.mode), self.user, self.group, self.size, self.path,
          self.isDir and '/' or "")
 
   def __repr__(self):
-    return "<WebHdfsStat %s>" % (self.path,)
+    return smart_str("<WebHdfsStat %s>" % (self.path,))
 
   def __getitem__(self, key):
     try: