Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Export Library fails due to httpx timeout #13

Closed
BlindWanderer opened this issue Feb 18, 2021 · 8 comments
Closed

Export Library fails due to httpx timeout #13

BlindWanderer opened this issue Feb 18, 2021 · 8 comments

Comments

@BlindWanderer
Copy link

BlindWanderer commented Feb 18, 2021

library export fails with large libraries or slow transfers due to use of default httpx request timeout (5 seconds).

A fix is required in the underlying library audible-cli uses and possibly a change to audible-cli as well.

See mkb79/Audible#36 for details

@BlindWanderer BlindWanderer changed the title Export Library Times out Export Library fails due to httpx timeout Feb 18, 2021
@mkb79
Copy link
Owner

mkb79 commented Feb 18, 2021

Your problem should be only happens, when exporting your library. There I forgot to add (or remove) a timeout. He takes the default from audible.Client.init() which is 10 seconds.

I have to choice if I add a higher timer to export library command only or to completely rebuild the package. What’s the best in your opinion?

@BlindWanderer
Copy link
Author

Rebuilding doesn't sound like fun! So I guess the first option? I'd have written more than a workaround if I had a good clean solution :(

@mkb79
Copy link
Owner

mkb79 commented Feb 18, 2021

I‘ve implement a timeout for library export command. Sorry for the delay. You can set a single request timeout for models.Library.get_from_api and models.Library.aget_from_api with the timeout request_params kwarg. The underlying audible.Client.get and audible.AsyncClient.get methods accept the timeout param and forward them to httpx.

@BlindWanderer
Copy link
Author

BlindWanderer commented Feb 20, 2021

#1a620c3 is a good start but causes the timeout to be passed in as a parameter of the request, not as a parameter to httpx. No worries, here's a fix for it.

I've tested the following change to models.py and it works:

    @classmethod
    async def aget_from_api(cls,
                            api_client: audible.AsyncClient,
                            locale: Optional[Locale] = None,
                            country_code: Optional[str] = None,
                            close_session: bool = False,
                            timeout = 10,
                            **request_params):

        if locale is not None and country_code is not None:
            raise ValueError("Locale and country_code provided. Expected only "
                             "one of them.")

        locale = Locale(country_code) if country_code else locale
        if locale:
            api_client.locale = locale

        if close_session:
            async with api_client:
                resp = await api_client.get("library", params=request_params, timeout=timeout)
        else:
            resp = await api_client.get("library", params=request_params, timeout=timeout)

        return cls(resp, auth=api_client.auth)

@mkb79
Copy link
Owner

mkb79 commented Feb 20, 2021

If you use an instance of audible.Client or audible.AsyncClient the code works how I wrote it.

The get, post, delete and put methods of them calls the self._split_kwargs() method to split special kwargs from params. But your code is much clearer for other.

@mkb79
Copy link
Owner

mkb79 commented Feb 20, 2021

I‘ve implement your changes in my code. Sorry for the delay. Had to hard work on my kindle repo.

@BlindWanderer
Copy link
Author

The speed at which you are responding to issues and comments is to be commended. It's much more typical for things to take months (and occasionally years) to be resolved. You do not need to apologize.

@BlindWanderer
Copy link
Author

Fixed in #558eb3b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants