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

Code before:
else:
    RETRYABLE_ERRORS = (httplib2.HttpLib2Error, IOError)

# Number of bytes to send/receive in each request.
CHUNKSIZE = 10 * 1024 * 1024

# Mimetype to use if one can't be guessed from the file extension.
DEFAULT_MIMETYPE = 'application/octet-stream'

# Time to sleep while waiting for eventual consistency to finish.
EVENTUAL_CONSISTENCY_SLEEP_INTERVAL = 0.1

# Maximum number of sleeps for eventual consistency.
EVENTUAL_CONSISTENCY_MAX_SLEEPS = 300

# Uri for batch requests
GCS_BATCH_URI = 'https://storage.googleapis.com/batch/storage/v1'


# Retry configuration. For more details, see https://tenacity.readthedocs.io/en/latest/
def custom_retry(retry_state):
    """custom_retry returns true if the outcome produced the function is retryable,
    false otherwise.
    """
    try:
        retry_state.outcome.result()
    except errors.HttpError as e:
        if e.resp < 500:
            raise
        return True
    except RETRYABLE_ERRORS:
        return True
    return False


gcs_retry = retry(retry=custom_retry,
                  wait=wait_exponential(multiplier=1, min=1, max=10),
                  stop=stop_after_attempt(5),
                  reraise=True,
                  after=after_log(logger, logging.WARNING))


def _wait_for_consistency(checker):
    """Eventual consistency: wait until GCS reports something is true.

    This is necessary for e.g. create/delete where the operation might return,
    but won't be reflected for a bit.
    """
    for _ in range(EVENTUAL_CONSISTENCY_MAX_SLEEPS):
        if checker():
            return

Review comment (suggestion):
I haven't used tenacity before, but I think this will return the actual result of the wrapped call, whereas you below return a boolean.

I think it can be expressed easier with retry_if_exception with a custom predicate, e.g.

```
def is_error_5xx(err: Exception):
    return isinstance(err, errors.HttpError) and e.resp >= 500

gcs_retry = retry(retry=retry_if_exception(is_error_5xx),
    ...

```


Code after:
    logger.warning("Loading GCS module without the python packages googleapiclient & google-auth. \
        This will crash at runtime if GCS functionality is used.")
else:
    RETRYABLE_ERRORS = (httplib2.HttpLib2Error, IOError)

# Number of bytes to send/receive in each request.
CHUNKSIZE = 10 * 1024 * 1024

# Mimetype to use if one can't be guessed from the file extension.
DEFAULT_MIMETYPE = 'application/octet-stream'

# Time to sleep while waiting for eventual consistency to finish.
EVENTUAL_CONSISTENCY_SLEEP_INTERVAL = 0.1

# Maximum number of sleeps for eventual consistency.
EVENTUAL_CONSISTENCY_MAX_SLEEPS = 300

# Uri for batch requests
GCS_BATCH_URI = 'https://storage.googleapis.com/batch/storage/v1'


# Retry configurations. For more details, see https://tenacity.readthedocs.io/en/latest/
def is_error_5xx(err):
    return isinstance(err, errors.HttpError) and err.resp.status >= 500


gcs_retry = retry(retry=(retry_if_exception(is_error_5xx) | retry_if_exception_type(RETRYABLE_ERRORS)),
                  wait=wait_exponential(multiplier=1, min=1, max=10),
                  stop=stop_after_attempt(5),
                  reraise=True,
                  after=after_log(logger, logging.WARNING))


def _wait_for_consistency(checker):
    """Eventual consistency: wait until GCS reports something is true.

    This is necessary for e.g. create/delete where the operation might return,
    but won't be reflected for a bit.
    """
    for _ in range(EVENTUAL_CONSISTENCY_MAX_SLEEPS):
        if checker():
            return

        time.sleep(EVENTUAL_CONSISTENCY_SLEEP_INTERVAL)

    logger.warning('Exceeded wait for eventual GCS consistency - this may be a'
                   'bug in the library or something is terribly wrong.')


class InvalidDeleteException(luigi.target.FileSystemException):
    pass