Put icons on the server side (client patch)

[ced, 2011-02-03 19:03:16]

http://codereview.appspot.com/4080053/diff/1/tryton/common/common.py
File tryton/common/common.py (right):

http://codereview.appspot.com/4080053/diff/1/tryton/common/common.py#newcode52
tryton/common/common.py:52: icons = rpc.execute('model', 'ir.ui.icon',
'search_read', [], 0, None,
You must catch exception and process it.

http://codereview.appspot.com/4080053/diff/1/tryton/common/common.py#newcode55
tryton/common/common.py:55: self._tryton_icons.append((icon['id'],
icon['name']))
Cache should be connection dependent (host, port, database)

http://codereview.appspot.com/4080053/diff/1/tryton/common/common.py#newcode63
tryton/common/common.py:63: icon_ref = (self._name2id[iconname], iconname)
The iconname should be enough as unique reference.

http://codereview.appspot.com/4080053/diff/1/tryton/common/common.py#newcode65
tryton/common/common.py:65: to_load =
self._tryton_icons[idx-self.batchnum/2:idx+self.batchnum/2]
idx-self.batchnum/2 could be negative which produce an unexpected behavior

http://codereview.appspot.com/4080053/diff/1/tryton/common/common.py#newcode67
tryton/common/common.py:67: icons = rpc.execute('model', 'ir.ui.icon', 'read',
ids,
You must catch exception and process it if possible.

http://codereview.appspot.com/4080053/diff/1/tryton/common/common.py#newcode71
tryton/common/common.py:71: svgfile = tempfile.NamedTemporaryFile(delete=False)
I think it will be more secure to use tempfile.mkstemp

http://codereview.appspot.com/4080053/diff/1/tryton/gui/window/wizard.py
File tryton/gui/window/wizard.py (left):

http://codereview.appspot.com/4080053/diff/1/tryton/gui/window/wizard.py#oldc...
tryton/gui/window/wizard.py:26: self.email_print = False
Why is it removed?

[ced, 2011-02-03 19:10:32]

http://codereview.appspot.com/4080053/diff/1/tryton/common/common.py
File tryton/common/common.py (right):

http://codereview.appspot.com/4080053/diff/1/tryton/common/common.py#newcode2
tryton/common/common.py:2: #this repository contains the full copyright notices
and license terms.
Must import with_statement from future

[nicoe, 2011-02-03 19:51:19]

http://codereview.appspot.com/4080053/diff/1/tryton/common/common.py
File tryton/common/common.py (right):

http://codereview.appspot.com/4080053/diff/1/tryton/common/common.py#newcode55
tryton/common/common.py:55: self._tryton_icons.append((icon['id'],
icon['name']))
On 2011/02/03 19:03:17, ced wrote:
> Cache should be connection dependent (host, port, database)

load_icons is launched after each login.

http://codereview.appspot.com/4080053/diff/1/tryton/common/common.py#newcode63
tryton/common/common.py:63: icon_ref = (self._name2id[iconname], iconname)
On 2011/02/03 19:03:17, ced wrote:
> The iconname should be enough as unique reference.

I prefer having a list and a dict for fast access than looping over on or the
other to reconstruct the data.

http://codereview.appspot.com/4080053/diff/1/tryton/common/common.py#newcode71
tryton/common/common.py:71: svgfile = tempfile.NamedTemporaryFile(delete=False)
On 2011/02/03 19:03:17, ced wrote:
> I think it will be more secure to use tempfile.mkstemp

It is not NamedTemporaryFile uses mkstemp.

http://codereview.appspot.com/4080053/diff/1/tryton/gui/window/wizard.py
File tryton/gui/window/wizard.py (left):

http://codereview.appspot.com/4080053/diff/1/tryton/gui/window/wizard.py#oldc...
tryton/gui/window/wizard.py:26: self.email_print = False
On 2011/02/03 19:03:17, ced wrote:
> Why is it removed?

I launched a macro to remove all "print " in the source I edited.

[ced, 2011-02-03 20:04:02]

http://codereview.appspot.com/4080053/diff/1/tryton/common/common.py
File tryton/common/common.py (right):

http://codereview.appspot.com/4080053/diff/1/tryton/common/common.py#newcode50
tryton/common/common.py:50: self._tryton_icons.pop()
You could use slice delete (faster):
del self._tryton_icons[:]

http://codereview.appspot.com/4080053/diff/1/tryton/common/common.py#newcode63
tryton/common/common.py:63: icon_ref = (self._name2id[iconname], iconname)
On 2011/02/03 19:51:19, nicoe wrote:
> On 2011/02/03 19:03:17, ced wrote:
> > The iconname should be enough as unique reference.
> 
> I prefer having a list and a dict for fast access than looping over on or the
> other to reconstruct the data.

I understand. But what I mean is that you can use iconname instead of
(self._name2id[iconname], iconname)

[nicoe, 2011-02-04 10:03:01]

http://codereview.appspot.com/4080053/diff/1/tryton/common/common.py
File tryton/common/common.py (right):

http://codereview.appspot.com/4080053/diff/1/tryton/common/common.py#newcode2
tryton/common/common.py:2: #this repository contains the full copyright notices
and license terms.
On 2011/02/03 19:10:32, ced wrote:
> Must import with_statement from future

Done.

http://codereview.appspot.com/4080053/diff/1/tryton/common/common.py#newcode50
tryton/common/common.py:50: self._tryton_icons.pop()
On 2011/02/03 20:04:02, ced wrote:
> You could use slice delete (faster):
> del self._tryton_icons[:]

Done.

http://codereview.appspot.com/4080053/diff/1/tryton/common/common.py#newcode65
tryton/common/common.py:65: to_load =
self._tryton_icons[idx-self.batchnum/2:idx+self.batchnum/2]
On 2011/02/03 19:03:17, ced wrote:
> idx-self.batchnum/2 could be negative which produce an unexpected behavior

Done.

http://codereview.appspot.com/4080053/diff/1/tryton/gui/window/wizard.py
File tryton/gui/window/wizard.py (left):

http://codereview.appspot.com/4080053/diff/1/tryton/gui/window/wizard.py#oldc...
tryton/gui/window/wizard.py:26: self.email_print = False
On 2011/02/03 19:51:19, nicoe wrote:
> On 2011/02/03 19:03:17, ced wrote:
> > Why is it removed?
> 
> I launched a macro to remove all "print " in the source I edited.

Done.

[ced, 2011-02-07 13:13:22]

http://codereview.appspot.com/4080053/diff/10001/tryton/common/common.py
File tryton/common/common.py (right):

http://codereview.appspot.com/4080053/diff/10001/tryton/common/common.py#newc...
tryton/common/common.py:56: except Exception, error:
error is not used.

http://codereview.appspot.com/4080053/diff/10001/tryton/common/common.py#newc...
tryton/common/common.py:74: except Exception, erro:
erro is not used

[ced, 2011-02-28 14:21:02]

LGTM