Skip to content

feat(auto-retry)#75

Open
martintamare wants to merge 10 commits into
OCA:masterfrom
alkivi-sas:auto-retry
Open

feat(auto-retry)#75
martintamare wants to merge 10 commits into
OCA:masterfrom
alkivi-sas:auto-retry

Conversation

@martintamare

Copy link
Copy Markdown

With the latest odoo I start to have a lot of HTTPError 429 Too Many Request.
What do you think of this implementation of an auto-retry ?

@martintamare

Copy link
Copy Markdown
Author

This is a basic implemention. I will try to add test and glue all the pieces together once I know I'm on the right way ;)

@yajo

yajo commented Jun 21, 2022

Copy link
Copy Markdown
Member

Don't you think an auto-retry might just increase the problem of too many requests?

@martintamare

Copy link
Copy Markdown
Author

The patch specifically look for 429 errors and sleep exponentially before retrying, so no I don't think so ;)

Comment thread odoorpc/rpc/jsonrpclib.py Outdated
Comment thread odoorpc/rpc/jsonrpclib.py Outdated
Comment thread odoorpc/rpc/jsonrpclib.py Outdated
@martintamare

Copy link
Copy Markdown
Author

Thanks for the correction, it's not ready yet thow. I will fetch your changes and add the glue with the root object Odoo

@martintamare

martintamare commented Jun 21, 2022

Copy link
Copy Markdown
Author

Added glue.
Not sure if I should go kwargs all the way down to the proxy itself. I stopped at the Connector.

@martintamare

Copy link
Copy Markdown
Author

I would also that setting autoretry to True by default can be good.
At least for saas customers like us who use this library in dozen of scripts ...
What do you think about that ?

Comment thread odoorpc/odoo.py
Comment thread odoorpc/rpc/__init__.py
Comment on lines +41 to +66
# Default autoretry for .odoo.com (saas) hosts
if 'autoretry' in kwargs:
self._autoretry = kwargs['autoretry']
elif host.endswith('.odoo.com'):
self._autoretry = True
else:
self._autoretry = False

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't particularly like this kind of magic. But in case it were, then you can default the autoretry arg to None, enabling this magic, insteado of relying in the existence of the kwarg. It would be the same result, but simplify the methods signature.

Making things obvious and predictable is good IMHO.

In any case, this (and the rest of the feature) deserves an entry in the docs.

@yajo yajo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some typos

Comment thread odoorpc/odoo.py Outdated
Comment thread odoorpc/rpc/__init__.py Outdated
Comment thread odoorpc/rpc/__init__.py Outdated
Comment thread odoorpc/rpc/jsonrpclib.py Outdated
martintamare and others added 5 commits May 20, 2024 13:15
Co-authored-by: Jairo Llopis <973709+yajo@users.noreply.github.com>
Co-authored-by: Jairo Llopis <973709+yajo@users.noreply.github.com>
Co-authored-by: Jairo Llopis <973709+yajo@users.noreply.github.com>
Co-authored-by: Jairo Llopis <973709+yajo@users.noreply.github.com>
Co-authored-by: Jairo Llopis <973709+yajo@users.noreply.github.com>
@martintamare martintamare requested a review from yajo May 20, 2024 11:25
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

Successfully merging this pull request may close these issues.

2 participants