Tryton Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1)

Issue 345941005: tryton-env: Add data limitation capabilities on inbound parameters of RPC calls

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 7 months ago by nicoe
Modified:
1 year, 2 months ago
Reviewers:
pokoli, reviewbot, ced, dave, albert
Visibility:
Public.

Description

Patch Set 1 #

Total comments: 10

Patch Set 2 : Add doc and take into account remarks #

Total comments: 29

Patch Set 3 : Fix remarks #

Total comments: 13

Patch Set 4 : Raise 413 and make function private #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+95 lines, -17 lines) Patch
M trytond/CHANGELOG View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M trytond/doc/ref/rpc.rst View 1 2 2 chunks +15 lines, -1 line 3 comments Download
M trytond/trytond/model/modelstorage.py View 1 2 3 chunks +47 lines, -14 lines 2 comments Download
M trytond/trytond/rpc.py View 1 2 3 4 chunks +32 lines, -2 lines 1 comment Download

Messages

Total messages: 22
nicoe
2 years, 7 months ago (2021-04-16 09:48:21 UTC) #1
reviewbot
flake8 OK URL: https://codereview.tryton.org/345941005
2 years, 7 months ago (2021-04-16 09:51:39 UTC) #2
dave
I've only had a quick look, and I'm not sure whether "rate" is the right ...
2 years, 7 months ago (2021-04-16 09:59:58 UTC) #3
pokoli
Mising documentation
2 years, 7 months ago (2021-04-16 10:04:08 UTC) #4
albert
https://codereview.tryton.org/345941005/diff/359511007/trytond/trytond/model/modelstorage.py File trytond/trytond/model/modelstorage.py (right): https://codereview.tryton.org/345941005/diff/359511007/trytond/trytond/model/modelstorage.py#newcode147 trytond/trytond/model/modelstorage.py:147: 'limit': lambda v: min(v or 100_000, 100_000), Why is ...
2 years, 7 months ago (2021-04-16 11:30:12 UTC) #5
nicoe
On 2021/04/16 09:59:58, dave wrote: > I've only had a quick look, and I'm not ...
2 years, 7 months ago (2021-04-16 17:11:32 UTC) #6
nicoe
https://codereview.tryton.org/345941005/diff/359511007/trytond/trytond/model/modelstorage.py File trytond/trytond/model/modelstorage.py (right): https://codereview.tryton.org/345941005/diff/359511007/trytond/trytond/model/modelstorage.py#newcode147 trytond/trytond/model/modelstorage.py:147: 'limit': lambda v: min(v or 100_000, 100_000), On 2021/04/16 ...
2 years, 7 months ago (2021-04-16 17:11:38 UTC) #7
ced
https://codereview.tryton.org/345941005/diff/359511007/trytond/trytond/model/modelstorage.py File trytond/trytond/model/modelstorage.py (right): https://codereview.tryton.org/345941005/diff/359511007/trytond/trytond/model/modelstorage.py#newcode130 trytond/trytond/model/modelstorage.py:130: 'ids': lambda v: v[:1_000_000], Should raise an ValueError. https://codereview.tryton.org/345941005/diff/359511007/trytond/trytond/model/modelstorage.py#newcode147 ...
2 years, 7 months ago (2021-04-19 11:41:31 UTC) #8
nicoe
2 years, 6 months ago (2021-05-10 16:37:24 UTC) #9
reviewbot
flake8 OK URL: https://codereview.tryton.org/345941005
2 years, 6 months ago (2021-05-10 16:58:42 UTC) #10
dave
Just a few minor suggestions https://codereview.tryton.org/345941005/diff/349861002/trytond/CHANGELOG File trytond/CHANGELOG (right): https://codereview.tryton.org/345941005/diff/349861002/trytond/CHANGELOG#newcode1 trytond/CHANGELOG:1: * Add data limitation ...
2 years, 6 months ago (2021-05-10 19:22:08 UTC) #11
ced
https://codereview.tryton.org/345941005/diff/349861002/trytond/trytond/model/modelstorage.py File trytond/trytond/model/modelstorage.py (right): https://codereview.tryton.org/345941005/diff/349861002/trytond/trytond/model/modelstorage.py#newcode33 trytond/trytond/model/modelstorage.py:33: _rpc_max_size = config.getint('request', 'argument_max_size') I think for consistency, it ...
2 years, 5 months ago (2021-07-07 11:35:37 UTC) #12
nicoe
https://codereview.tryton.org/345941005/diff/359511007/trytond/trytond/model/modelstorage.py File trytond/trytond/model/modelstorage.py (right): https://codereview.tryton.org/345941005/diff/359511007/trytond/trytond/model/modelstorage.py#newcode130 trytond/trytond/model/modelstorage.py:130: 'ids': lambda v: v[:1_000_000], On 2021/04/19 11:41:31, ced wrote: ...
1 year, 5 months ago (2022-07-05 17:13:25 UTC) #13
nicoe
1 year, 5 months ago (2022-07-05 17:14:20 UTC) #14
reviewbot
https://codereview.tryton.org/345941005/diff/441261003/trytond/trytond/model/modelstorage.py#newcode23 trytond/trytond/model/modelstorage.py:23: 'trytond.rpc.truncate' imported but unused URL: https://codereview.tryton.org/345941005
1 year, 5 months ago (2022-07-05 17:38:55 UTC) #15
ced
Missing update of doc/topics/configuration.rst https://codereview.tryton.org/345941005/diff/349861002/trytond/trytond/model/modelstorage.py File trytond/trytond/model/modelstorage.py (right): https://codereview.tryton.org/345941005/diff/349861002/trytond/trytond/model/modelstorage.py#newcode33 trytond/trytond/model/modelstorage.py:33: _rpc_max_size = config.getint('request', 'argument_max_size') On ...
1 year, 2 months ago (2022-09-11 22:12:36 UTC) #16
ced
https://codereview.tryton.org/345941005/diff/441261003/trytond/trytond/rpc.py File trytond/trytond/rpc.py (right): https://codereview.tryton.org/345941005/diff/441261003/trytond/trytond/rpc.py#newcode108 trytond/trytond/rpc.py:108: raise ValueError I think it should raise 413 http ...
1 year, 2 months ago (2022-09-11 22:34:02 UTC) #17
nicoe
https://codereview.tryton.org/345941005/diff/441261003/trytond/trytond/rpc.py File trytond/trytond/rpc.py (right): https://codereview.tryton.org/345941005/diff/441261003/trytond/trytond/rpc.py#newcode93 trytond/trytond/rpc.py:93: def check_limitations(self, args, kwargs): On 2022/09/11 22:12:35, ced wrote: ...
1 year, 2 months ago (2022-09-26 15:11:06 UTC) #18
nicoe
1 year, 2 months ago (2022-09-26 15:11:49 UTC) #19
ced
https://codereview.tryton.org/345941005/diff/441261003/trytond/trytond/rpc.py File trytond/trytond/rpc.py (right): https://codereview.tryton.org/345941005/diff/441261003/trytond/trytond/rpc.py#newcode93 trytond/trytond/rpc.py:93: def check_limitations(self, args, kwargs): On 2022/09/26 15:11:06, nicoe wrote: ...
1 year, 2 months ago (2022-09-26 15:13:37 UTC) #20
reviewbot
https://codereview.tryton.org/345941005/diff/433521003/trytond/trytond/model/modelstorage.py#newcode23 trytond/trytond/model/modelstorage.py:23: 'trytond.rpc.truncate' imported but unused URL: https://codereview.tryton.org/345941005
1 year, 2 months ago (2022-09-26 15:32:35 UTC) #21
ced
1 year, 2 months ago (2022-09-26 21:42:53 UTC) #22
https://codereview.tryton.org/345941005/diff/433521003/trytond/doc/ref/rpc.rst
File trytond/doc/ref/rpc.rst (right):

https://codereview.tryton.org/345941005/diff/433521003/trytond/doc/ref/rpc.rs...
trytond/doc/ref/rpc.rst:45: .. attribute:: RPC.data_limitations
must be updated

https://codereview.tryton.org/345941005/diff/433521003/trytond/doc/ref/rpc.rs...
trytond/doc/ref/rpc.rst:52: * A number: the function will be applied to the
positional argument at
Use present form

https://codereview.tryton.org/345941005/diff/433521003/trytond/doc/ref/rpc.rs...
trytond/doc/ref/rpc.rst:52: * A number: the function will be applied to the
positional argument at
It is no more a function.

https://codereview.tryton.org/345941005/diff/433521003/trytond/trytond/model/...
File trytond/trytond/model/modelstorage.py (right):

https://codereview.tryton.org/345941005/diff/433521003/trytond/trytond/model/...
trytond/trytond/model/modelstorage.py:39: _rpc_max_size =
config.getint('request', 'argument_max_size')
I think it should be named: max_size_argument to follow existing pattern.

https://codereview.tryton.org/345941005/diff/433521003/trytond/trytond/model/...
trytond/trytond/model/modelstorage.py:39: _rpc_max_size =
config.getint('request', 'argument_max_size')
Missing documentation.

https://codereview.tryton.org/345941005/diff/433521003/trytond/trytond/rpc.py
File trytond/trytond/rpc.py (right):

https://codereview.tryton.org/345941005/diff/433521003/trytond/trytond/rpc.py...
trytond/trytond/rpc.py:27: data_limitations: A dictionary of functions used to
impose limits
not more valid
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld d9ca037-tainted