Repository: spotify/luigi
File: luigi/contrib/s3.py (Python)

Code before:

        (src_bucket, src_key) = self._path_to_bucket_and_key(source_path)
        (dst_bucket, dst_key) = self._path_to_bucket_and_key(destination_path)

        # don't allow threads to be less than 3
        threads = 3 if threads < 3 else threads
        from boto3.s3.transfer import TransferConfig

        transfer_config = TransferConfig(max_concurrency=threads, multipart_chunksize=part_size)

        if self.isdir(source_path):
            return self._copy_dir(src_bucket, src_key, dst_bucket, dst_key, source_path,
                                  transfer_config, threads=100, start_time=start_time, end_time=end_time, **kwargs)

        # If the file isn't a directory just perform a simple copy
        else:
            item = self.get_key(source_path)
            self._copy_file(src_bucket, src_key, dst_bucket, dst_key, transfer_config, **kwargs)
            return 1, item.size

    def _copy_file(self, src_bucket, src_key, dst_bucket, dst_key, transfer_config, **kwargs):
        copy_source = {
            'Bucket': src_bucket,
            'Key': src_key
        }

        self.s3.meta.client.copy(copy_source, dst_bucket, dst_key, Config=transfer_config, ExtraArgs=kwargs)

    def _copy_dir(self, src_bucket, src_key, dst_bucket, dst_key, source_path, transfer_config,
                  threads=100, start_time=None, end_time=None, **kwargs):
        start = datetime.datetime.now()
        copy_jobs = []
        management_pool = ThreadPool(processes=threads)
        (bucket, key) = self._path_to_bucket_and_key(source_path)
        key_path = self._add_path_delimiter(key)
        key_path_len = len(key_path)
        src_prefix = self._add_path_delimiter(src_key)
        dst_prefix = self._add_path_delimiter(dst_key)
        total_size_bytes = 0
        total_keys = 0
        for item in self.list(source_path, start_time=start_time, end_time=end_time, return_key=True):
            path = item.key[key_path_len:]
            # prevents copy attempt of empty key in folder
            if path != '' and path != '/':
                total_keys += 1
                total_size_bytes += item.size
                extra_args = {'ExtraArgs': kwargs} if kwargs else {}
                job = management_pool.apply_async(self._copy_file,
                                                  args=(src_bucket, src_prefix + path, dst_bucket, dst_prefix + path,
                                                        transfer_config),
                                                  kwds=extra_args)

Review comment (suggestion):
It is still weird to be that `_copy_dir()` and `_copy_file()` don't return the same thing. `_copy_dir()` returns a tuple and `_copy_file()` returns `None`

Code after:
        :param kwargs: Keyword arguments are passed to the boto function `copy` as ExtraArgs
        :returns tuple (number_of_files_copied, total_size_copied_in_bytes)
        """

        # don't allow threads to be less than 3
        threads = 3 if threads < 3 else threads

        if self.isdir(source_path):
            return self._copy_dir(source_path, destination_path, threads=threads,
                                  start_time=start_time, end_time=end_time, part_size=part_size, **kwargs)

        # If the file isn't a directory just perform a simple copy
        else:
            return self._copy_file(source_path, destination_path, threads=threads, part_size=part_size, **kwargs)

    def _copy_file(self, source_path, destination_path, threads=DEFAULT_THREADS, part_size=DEFAULT_PART_SIZE, **kwargs):
        src_bucket, src_key = self._path_to_bucket_and_key(source_path)
        dst_bucket, dst_key = self._path_to_bucket_and_key(destination_path)
        transfer_config = TransferConfig(max_concurrency=threads, multipart_chunksize=part_size)
        item = self.get_key(source_path)
        copy_source = {
            'Bucket': src_bucket,
            'Key': src_key
        }

        self.s3.meta.client.copy(copy_source, dst_bucket, dst_key, Config=transfer_config, ExtraArgs=kwargs)

        return 1, item.size

    def _copy_dir(self, source_path, destination_path, threads=DEFAULT_THREADS,
                  start_time=None, end_time=None, part_size=DEFAULT_PART_SIZE, **kwargs):
        start = datetime.datetime.now()
        copy_jobs = []
        management_pool = ThreadPool(processes=threads)
        transfer_config = TransferConfig(max_concurrency=threads, multipart_chunksize=part_size)
        src_bucket, src_key = self._path_to_bucket_and_key(source_path)
        dst_bucket, dst_key = self._path_to_bucket_and_key(destination_path)
        src_prefix = self._add_path_delimiter(src_key)
        dst_prefix = self._add_path_delimiter(dst_key)
        key_path_len = len(src_prefix)
        total_size_bytes = 0
        total_keys = 0
        for item in self.list(source_path, start_time=start_time, end_time=end_time, return_key=True):
            path = item.key[key_path_len:]
            # prevents copy attempt of empty key in folder
            if path != '' and path != '/':
                total_keys += 1
                total_size_bytes += item.size
                copy_source = {
                    'Bucket': src_bucket,
                    'Key': src_prefix + path