Coding Guidelines

[bpepple]

I was looking to improve your API calls code (in particular the rate limiting and exception handling), and don't see any formatting style or linting being used for the project. Is there any type of guidance you could give? Pep8 fine?

[allaboutduncan]

Yep, using Pep8 and Ruff internally.

Just setup setup Super-Linter on all PRs yesterday as well.

Assuming Metron related API calls? Let me know anything in particular that you're seeing that's problematic.

[bpepple]

Yeah, the rate limiting and error handling would be greatly improved by making use of Mokkari's exceptions sorta like this:

import mokkari
from mokkari.exceptions import RateLimitError
import time

m = mokkari.api(username, password)

try:
    issue = m.issue(31660)
except RateLimitError as e:
    # Display user-friendly message
    print(f"Rate limited: {e}")

    # Programmatically wait for the exact time needed
    print(f"Waiting {e.retry_after} seconds...")
    time.sleep(e.retry_after)

    # Retry the request
    issue = m.issue(31660)

Or possibly a decorator function:

# Rate limit threshold in seconds - values above this are considered daily limits
DAILY_RATE_LIMIT_THRESHOLD = 60


class DailyRateLimitError(Exception):
    """Raised when the daily rate limit has been reached."""


def handle_rate_limit(func):
    """
    Decorator to handle Mokkari rate limit errors.

    If retry_after > 60 seconds, raises DailyRateLimitError.
    Otherwise, waits for retry_after seconds and retries once.
    """

    @wraps(func)
    def wrapper(*args, **kwargs):
        try:
            return func(*args, **kwargs)
        except RateLimitError as e:
            retry_after = getattr(e, "retry_after", 0)

            if retry_after > DAILY_RATE_LIMIT_THRESHOLD:
                logger.warning("Daily rate limit reached. Retry after %s seconds.", retry_after)
                raise DailyRateLimitError(retry_after) from e

            # Wait and retry once
            logger.info("Rate limited. Waiting %s seconds before retrying...", retry_after)
            time.sleep(retry_after)

            try:
                return func(*args, **kwargs)
            except RateLimitError as retry_error:
                # If we hit rate limit again, check if it's the daily limit
                retry_after = getattr(retry_error, "retry_after", 0)
                if retry_after > DAILY_RATE_LIMIT_THRESHOLD:
                    raise DailyRateLimitError(retry_after) from retry_error
                raise

    return wrapper

I'll need to look at the code more closely to see what would work best.

[allaboutduncan]

General Metron model is in
https://github.com/allaboutduncan/clu-comics/blob/main/models/metron.py

and the provider to get metadata is at
https://github.com/allaboutduncan/clu-comics/blob/main/models/providers/metron_provider.py

[allaboutduncan]

I can drop the decorator into my Metron Provider class pretty quickly. Give me a few and I'll have a PR out.

I've been running "house cleaning" tasks the past 2 days

[allaboutduncan]

@bpepple see what you think

Implement Mokkari Rate Limiter and Add to all Metron Calls #108

[bpepple]

So, I went about it differently by adding retry logic and dropped some unnecessary code (is_mokkari_available, generic Exceptions, etc), and refactored some duplicate code. Here's what I've done so far:

https://github.com/bpepple/clu-comics/tree/dev

You can decide which route you want to go with. One thing that I'd suggest is adding some tests since it would identify any function breakage.

Haven't looked at the providers very closely since they don't use Mokkari directly, but there are probably some changes that could be made there to make it easier to maintain.

[allaboutduncan]

Tests are next on my list and I'll review this tomorrow.

Thanks for the help

[bpepple]

It not quite ready yet. I'll open a PR when I'm done and you can decide whether you want to use it not.