Missing callable check for progress_func

[maxstrobel]

In artifactory.py a check to validate if progress_func is given, is missing - if progress_func is None a TypeError (TypeError: 'NoneType' object is not callable) is raised.

def writeto(self, pathobj, file, chunk_size, progress_func):
    """
    Downloads large file in chunks and prints progress
    :param pathobj: path like object
    :param file: IO object
    :param chunk_size: chunk size in bytes, recommend. eg 1024*1024 is 1Mb
    :param progress_func: Provide custom function to print out or suppress print by setting to None
    :return: None
    """

    response = self.get_response(pathobj)
    file_size = int(response.headers.get("Content-Length", 0))
    bytes_read = 0
    real_chunk = 0
    for chunk in response.iter_content(chunk_size=chunk_size):
        real_chunk += len(chunk)
        if callable(progress_func) and real_chunk - chunk_size >= 0:
            # Artifactory archives folders on fly and can reduce requested chunk size to 8kB, thus report
            # only when real chunk size met
            bytes_read += real_chunk
            real_chunk = 0
            progress_func(bytes_read, file_size)

        file.write(chunk)

    if real_chunk > 0:
        progress_func(bytes_read + real_chunk, file_size)

https://github.com/devopshq/artifactory/blob/1b2d87bc8c249ebea81dda10349b329efe74b48c/artifactory.py#L1400

[mrdalrym]

This breaks backwards compatibility when explicitly specifying progress_func=None. It shows there in the documentation: :param progress_func: Provide custom function to print out or suppress print by setting to None

Maybe this should have been if progress_func and callable(progress_func) ...?

[beliaev-maksim]

@mrdalrym I don't understand your point, there is already a PR that fixes it What is the issue that you see?

[allburov]

@mrdalrym the version with the fix 0.8.3, could you have look at it?

[mrdalrym]

My bad, I just saw stuff failing on my end, saw this as a last change and was around when the 0.8.2 release went out. But looks is callable(None) will handle that case (i.e. this was the fix for that issue). Thanks for fixing this fast!