Ct 1311 cherrypick unicode fixes

conductor/lib/file_utils.py

@@ -116,16 +116,17 @@ def process_upload_filepaths(paths):
 
 def process_upload_filepath(path, strict=True):
     '''
-    Process the given path to ensure that the path is valid (exists on disk),
-    and return any/all files which the path may represent.
-    For example, if the path is a directory or an image sequence, then explicitly
-    list and return all files that that path represents/contains.
+    Process the given path to ensure that the path is valid: (Exists on disk AND
+    does not contain Unicode characters). Return any/all files the path may
+    represent. For example, if the path is a directory or image sequence, then
+    explicitly list and return all files that that path represents/contains.
 
 
-    strict: bool. When True and the give path does not exist on disk, raise an
+    strict: bool. When True and the path is not valid, raise an
                   exception.
                   Note that when this function is given a directory path, and
                   and it finds any broken symlinks within the directory, the
+                  recursion is not strict. See comment below.
 
 
     This function should be able to handle various types of paths:
@@ -136,19 +137,19 @@ def process_upload_filepath(path, strict=True):
 
     Process the path by doing the following:
 
-    1. If the path is  an image sequence notation, "explode" it and return
-        each frame's filepath.  This relies on the file
-        actually being on disk, as the underlying call is to glob.glob(regex).
-        Validate that there is at least one frame on disk for the image sequence.
-        There is no 100% reliable way to know how many frames should actually be
-        part of the image sequence, but we can at least validate that there is
-        a single frame.
+    1. If the path is an image sequence notation, "explode" it and return each
+        frame's filepath.  This relies on the file actually being on disk, as
+        the underlying call is to glob.glob(regex). Validate that there is at
+        least one frame on disk for the image sequence. There is no 100%
+        reliable way to know how many frames should actually be part of the
+        image sequence, but we can at least validate that there is a single
+        frame.
 
     2. If the path is a directory then recursively add all file/dir paths
         contained within it
 
-    3. If the path is a file then ensure that it exists on disk and that it conforms
-       to Conductor's expectations.
+    3. If the path is a file then ensure that it exists on disk and that it
+       conforms to Conductor's expectations.
 
 
 
@@ -157,6 +158,15 @@ def process_upload_filepath(path, strict=True):
     paths = []
 
     if path:
+        try:
+            str(path)
+        except UnicodeEncodeError:
+            message = "Unicode filenames are not supported: %s" % path
+            # TODO: Figure out if this exception should always raise (if so,
+            # remove if strict:)
+            if strict:
+                raise exceptions.InvalidPathException(message)
+            logger.warning(message)
 
         # If the path is a file (and it exits)
         if os.path.isfile(path):

conductor/lib/uploader.py

@@ -33,13 +33,13 @@ def do_work(self, job, thread_int):
         logger.debug('job is %s', job)
         filename, submission_time_md5 = job
         assert isinstance(filename, (str, unicode)), "Filepath not of expected type. Got %s" % type(filename)
-        filename = str(filename)
+        encoded_filename = filename.encode("utf8")
         current_md5 = self.get_md5(filename)
         # if a submission time md5 was provided then check against it
         if submission_time_md5:
-            logger.info("Enforcing md5 match: %s for: %s", submission_time_md5, filename)
+            logger.info("Enforcing md5 match: %s for: %s", submission_time_md5, encoded_filename)
             if current_md5 != submission_time_md5:
-                message = 'MD5 of %s has changed since submission\n' % filename
+                message = 'MD5 of %s has changed since submission\n' % encoded_filename
                 message += 'submitted md5: %s\n' % submission_time_md5
                 message += 'current md5:   %s\n' % current_md5
                 message += 'This is likely due to the file being written to after the user submitted the job but before it got uploaded to conductor'

tests/fixtures/upload_file1

@@ -0,0 +1 @@
+fixtures/single_file

tests/fixtures/upload_file1asdfasdf

@@ -0,0 +1 @@
+fixtures/single_file

tests/fixtures/upload_file2

@@ -0,0 +1 @@
+fixtures/single_file,fixtures/one_symlink

tests/upload_helpers/upload_file3 → tests/fixtures/upload_file3

@@ -1 +1 @@
-/Users/martin/Dropbox/conductor/src/conductor_ae/src/client/tests/upload_helpers/single_file/foo
+/Users/martin/Dropbox/conductor/src/conductor_ae/src/client/tests/fixtures/single_file/foo

tests/test_file_utils.py

@@ -2,29 +2,42 @@
 
    isort:skip_file
 """
+import os
 
 import unittest
 import conductor.lib.file_utils as futil
 import logging
 
 
+fixtures_dir = os.path.join(os.path.dirname(os.path.realpath(__file__)), "fixtures")
+
+
 class ProcessDependenciesTest(unittest.TestCase):
+    def setUp(self):
+        logger = logging.getLogger("conductor")
+        logger.setLevel("DEBUG")
+
     def test_regular_filename(self):
         paths = ["/path/to/filename.txt"]
         deps = futil.process_dependencies(paths)
-        self.assertTrue("/path/to/filename.txt" in deps)
+        self.assertIn("/path/to/filename.txt", deps)
 
     def test_it_encodes_unicode_chars_in_error_message(self):
         # make sure logging is triggered as it should also encode unicode.
-        logger = logging.getLogger("conductor")
-        logger.setLevel("DEBUG")
-
         paths = [u"/path/to/\u0123/name.txt"]
         deps = futil.process_dependencies(paths)
-        self.assertTrue(u"/path/to/\u0123/name.txt" in deps)
-        self.assertTrue(
-            "/path/to/\xc4\xa3/name.txt" in deps[u"/path/to/\u0123/name.txt"]
-        )
+        self.assertIn(u"/path/to/\u0123/name.txt", deps)
+        self.assertIn("/path/to/\xc4\xa3/name.txt", deps[u"/path/to/\u0123/name.txt"])
+
+    def test_catches_existing_unicode_files(self):
+        unicode_dir = unicode(os.path.join(fixtures_dir, "unicode_files"), "utf8")
+        filenames = os.listdir(unicode_dir)
+        paths = [
+            os.path.join(unicode_dir, f) for f in filenames if not f.startswith(".")
+        ]
+        deps = futil.process_dependencies(paths)
+        for key in deps:
+            self.assertIn("Unicode filenames are not supported", deps[key])
 
 
 if __name__ == "__main__":

tests/test_uploads.py

@@ -19,7 +19,7 @@
 base_dir = os.path.dirname(test_dir)
 sys.path.append(base_dir)
 
-upload_test_helpers = os.path.join(test_dir,'upload_helpers')
+upload_test_helpers = os.path.join(test_dir,'fixtures')
 
 os.environ['FLASK_CONF'] = 'TEST' # disable retries n' stuff
 # os.environ['CONDUCTOR_DEVELOPMENT'] = '1' # uncomment this line to get debug messages