flask_tryton: Retry all request on any DatabaseOperationError

bug10

[ced, 2019-11-26 15:13:05]

I'm wondering if would not be better to decorate the all wrapper with a retry
loop.

https://codereview.appspot.com/565260043/diff/573290043/flask_tryton.py
File flask_tryton.py (right):

https://codereview.appspot.com/565260043/diff/573290043/flask_tryton.py#newco...
flask_tryton.py:125: with Transaction().start(database, transaction_user,
It could happen also here.

[pokoli, 2019-11-26 19:47:19]

Add retry loop on all the wrapper

[ced, 2019-11-26 21:01:10]

https://codereview.appspot.com/565260043/diff/553220043/flask_tryton.py
File flask_tryton.py (right):

https://codereview.appspot.com/565260043/diff/553220043/flask_tryton.py#newco...
flask_tryton.py:160: retry = int(config['retry'])
I think retry could be stored outside the method because it is static once the
app is initiated.

https://codereview.appspot.com/565260043/diff/553220043/flask_tryton.py#newco...
flask_tryton.py:164: return dispatch(func, args, kwargs)
I would prefer to have a decorator then splitting functions.

[pokoli, 2019-11-27 16:55:41]

Make retry global and use decorator

[ced, 2019-11-27 18:55:36]

I think we need a changelog entry.

https://codereview.appspot.com/565260043/diff/555030043/flask_tryton.py
File flask_tryton.py (right):

https://codereview.appspot.com/565260043/diff/555030043/flask_tryton.py#newco...
flask_tryton.py:31: retry = config.getint('database', 'retry')
This can not be parsed at module import because config file has not yet been
loaded.

https://codereview.appspot.com/565260043/diff/555030043/flask_tryton.py#newco...
flask_tryton.py:97: def retry_transaction(func):
I think this method could be at top level and take the retry as parameter.

https://codereview.appspot.com/565260043/diff/555030043/flask_tryton.py#newco...
flask_tryton.py:104: if count:
I think we should keep the is_readonly test. The value could be retrieved from
the Transaction instance.

https://codereview.appspot.com/565260043/diff/555030043/flask_tryton.py#newco...
flask_tryton.py:111: @wraps(func)
I'm wondering if it is the proper decorator order.

[pokoli, 2019-11-28 17:09:40]

Fix remarks

[pokoli, 2019-11-28 17:14:00]

https://codereview.appspot.com/565260043/diff/555030043/flask_tryton.py
File flask_tryton.py (right):

https://codereview.appspot.com/565260043/diff/555030043/flask_tryton.py#newco...
flask_tryton.py:31: retry = config.getint('database', 'retry')
On 2019/11/27 18:55:35, ced wrote:
> This can not be parsed at module import because config file has not yet been
> loaded.

Done.

https://codereview.appspot.com/565260043/diff/555030043/flask_tryton.py#newco...
flask_tryton.py:97: def retry_transaction(func):
On 2019/11/27 18:55:35, ced wrote:
> I think this method could be at top level and take the retry as parameter.

Done.

https://codereview.appspot.com/565260043/diff/555030043/flask_tryton.py#newco...
flask_tryton.py:104: if count:
On 2019/11/27 18:55:35, ced wrote:
> I think we should keep the is_readonly test. The value could be retrieved from
> the Transaction instance.

Done.

https://codereview.appspot.com/565260043/diff/555030043/flask_tryton.py#newco...
flask_tryton.py:111: @wraps(func)
On 2019/11/27 18:55:35, ced wrote:
> I'm wondering if it is the proper decorator order.

I've done some testing and yes it is.

[ced, 2019-11-28 23:17:55]

https://codereview.appspot.com/565260043/diff/555040043/CHANGELOG
File CHANGELOG (right):

https://codereview.appspot.com/565260043/diff/555040043/CHANGELOG#newcode1
CHANGELOG:1: * Add retry transacation decorator
transaction

https://codereview.appspot.com/565260043/diff/555040043/CHANGELOG#newcode1
CHANGELOG:1: * Add retry transacation decorator
I think the changelog is more about retrying all the request on any Database
Opration error

https://codereview.appspot.com/565260043/diff/555040043/flask_tryton.py
File flask_tryton.py (right):

https://codereview.appspot.com/565260043/diff/555040043/flask_tryton.py#newco...
flask_tryton.py:73: def retry_transaction(self, retry=None):
For it does not need to be an instance method but just a function in the module.

[pokoli, 2019-11-30 17:33:30]

Make decorator a function and update changelog

[ced, 2019-11-30 22:30:55]

LGTM