First, we need a syntax to represent data that is indepedent of Python and
that can handle list, dictionnary, string, number etc.
We thought about json which seems to be implemented in many languages and is
extendable.
Here is some examples:
A domain from ir.action.act_window:
[('parent', '=', False)] => [{'__class__': 'eval', 'v': "['parent', '=', False]"}]
A context from ir.action.act_window:
{'company': company} => {'company': {'__class__': 'eval', 'v': "company or False"}}
A digits from a Float:
(16, currency_digits) => {'__class__': 'eval', 'v': "[16, currency_digits]"}
A states like currency in account.invoice:
bool(lines) => {'__class__': 'eval', 'v': "len(lines) and True or False"}
As you can see, we define a new class of object "eval" which represents the strings
that must be evaluated on the client side.
Second, we need an easy and simple syntax that can be implemented in other
languages. So we will restrict it to the use of:
Type:
str, int/long, list
Operators:
and, or
Functions:
contains, getitem, len
Functions could be added later depending of the needs.
--
Cédric Krier
B2CK SPRL
Rue de Rotterdam, 4
4000 Liège
Belgium
Tel: +32 472 54 46 59
Email: cedric...@b2ck.com
Jabber: cedric...@b2ck.com
Website: http://www.b2ck.com/
twitter: http://twitter.com/cedrickrier
identi.ca: http://identi.ca/cedrickrier
> On Dec 14, 4:15 pm, Cédric Krier <cedric.kr...@b2ck.com> wrote:
I'm not quite sure what exactly you are proposing. Are you proposing to
change the domain specific language as the user/programmer writes it or
as it is passed to the client?
If you are proposing to change the language as passed to the client, I'
with you. We should be language independent here.
I vote against changing the language as the user/programmer writes it in
such a way as you both propose for some reason: Programmers have to
learn yet another language, we gain no benefit here and the current
language is easily parsed in python.
>> [{'__class__': 'eval', 'v': "['parent', '=', False]"}]
>> {'company': {'__class__': 'eval', 'v': "company or False"}}
>> {'__class__': 'eval', 'v': "[16, currency_digits]"}
>> {'__class__': 'eval', 'v': "len(lines) and True or False"}
This is terrible unreadable! We should not expose this to the user nor
to the programmer. It looks ugly and is redundant.
bch schrieb:
> Another solution is to use a list representation of those strings
> (inspired by Scheme syntax), example here: http://dpaste.org/ZJey/
Maybe we could extend your idea towards some function-oriented style.
This should allow a very simple implementation on almost every language
suporting some kind of 'evel()'. In Python this could be implemented
quite easily like in the example: http://dpaste.org/kIZy/
IMHO this a a bit more readable than bch proposal since it tights the
arguments towards function.
--
Schönen Gruß - Regards
Hartmut Goebel
Dipl.-Informatiker (univ.), CISSP, CSSLP
Goebel Consult
Spezialist für IT-Sicherheit in komplexen Umgebungen
http://www.goebel-consult.de
Monatliche Kolumne: http://www.cissp-gefluester.de/
Goebel Consult mit Mitglied bei http://www.7-it.de
both
>
> If you are proposing to change the language as passed to the client, I'
> with you. We should be language independent here.
>
> I vote against changing the language as the user/programmer writes it in
> such a way as you both propose for some reason: Programmers have to
> learn yet another language, we gain no benefit here and the current
> language is easily parsed in python.
>
> >> [{'__class__': 'eval', 'v': "['parent', '=', False]"}]
> >> {'company': {'__class__': 'eval', 'v': "company or False"}}
> >> {'__class__': 'eval', 'v': "[16, currency_digits]"}
> >> {'__class__': 'eval', 'v': "len(lines) and True or False"}
>
> This is terrible unreadable! We should not expose this to the user nor
> to the programmer. It looks ugly and is redundant.
There is no redundant.
>
> bch schrieb:
>
> > Another solution is to use a list representation of those strings
> > (inspired by Scheme syntax), example here: http://dpaste.org/ZJey/
>
> Maybe we could extend your idea towards some function-oriented style.
> This should allow a very simple implementation on almost every language
> suporting some kind of 'evel()'. In Python this could be implemented
> quite easily like in the example: http://dpaste.org/kIZy/
I don't understand as you say you don't want to change the language.
--
Cédric Krier
B2CK SPRL
Rue de Rotterdam, 4
4000 Liège
Belgium
Tel: +32 472 54 46 59
Email: cedric...@b2ck.com
Jabber: cedric...@b2ck.com
> Nice improvement. I forgot to say that with my version, because there is
> no need of eval anymore, there is no more risk of code injection (even
> if the current eval usage has been secured).
ACK. Instead of eval() we could use a simple implementation based on the
Python modules parser and ast*). These will give us a tree we just need
to walk the tree pre-order.
*) I'm not sure whether these modules are available in Python 2.4, but I
think so. Otherwise we could implement a simple parser using pyparsing,
which is already used for Tryton.
>>>> [{'__class__': 'eval', 'v': "['parent', '=', False]"}]
>>>> {'company': {'__class__': 'eval', 'v': "company or False"}}
>>>> {'__class__': 'eval', 'v': "[16, currency_digits]"}
>>>> {'__class__': 'eval', 'v': "len(lines) and True or False"}
>> This is terrible unreadable! We should not expose this to the user nor
>> to the programmer. It looks ugly and is redundant.
>
> There is no redundant.
From my point of view, there is a lot of redundancy:
{'__class__': 'eval', 'v':
is in every line.
>> Maybe we could extend your idea towards some function-oriented style.
>> This should allow a very simple implementation on almost every language
>> suporting some kind of 'evel()'. In Python this could be implemented
>> quite easily like in the example: http://dpaste.org/kIZy/
>
> I don't understand as you say you don't want to change the language.
I do not want to change the language, the user/programmer uses. But I
understood that we need to change the parameters passed to the clients,
since this needs to be language-independent.
My proposal at http://dpaste.org/kIZy/ is meant as a domain specific
language (DSL) we pass to the client. The user/programmer still writes
the same language which is translated it into the DSL by trytond.
It is a change because for now we use full python syntax.
I don't agree on the fact that the server must parse the code. It will
overload it for no benefit for the current implementation.
And if it is needed to parse the python code, I would prefer to have it on
client side, it will scale better. And if we keep python code, we don't need
to implement new parsing/VM in the client nor in the server as we still use
safe_eval.
And we could not ask people to write a complete python VM to evaluate those
strings (AST). We should limit to some functionnalities as I said in previous
emails.
And with simple python statement, they can be transform easily into many other
language statements with simple string replace.
--
Cédric Krier
B2CK SPRL
Rue de Rotterdam, 4
4000 Liège
Belgium
Tel: +32 472 54 46 59
Email: cedric...@b2ck.com
Jabber: cedric...@b2ck.com
> I don't agree on the fact that the server must parse the code. It will
> overload it for no benefit for the current implementation.
Indeed I've not been clear enough. My idea was to keep the current
language in the XML-Files. When the file is loaded, this code in
transformed into some "internal" representation (e.g. like bechamel
proposed) an this is passed to the client for evaluation.
But I understood that you prefer a single language. Unfortunately this
will force users to change the XML-Files.
> And we could not ask people to write a complete python VM to evaluate those
> strings (AST). We should limit to some functionnalities as I said in previous
> emails.
I never meant this. The DSL should be simply as it could be. This will
allow using existing parsers and tree-builders, no mater which language
the client is implemented in. These should exist in every language which
is capable of implementing a Tryton client. This would not require
implementing a python VM in another language.
For our *Python* implementation of the Tryton client, the cheapes way to
get a parse-tree would be the parser module. We only need to implement
some walker (as bechamel showed) and all the lexing, parsing and
tree-building would be done by parser :-)
> And with simple python statement, they can be transform easily into many other
> language statements with simple string replace.
Unfortunately experience shows that string replace is not able to
convert code from one language to another.*) Either die replacement
patterns are much to complicated (and error-frown) or you will
If our DSL is function oriented (as I proposed), some out-of-the-box
parser could be used in any language and some simple tree walker (like
the one bechamel showed) is implemented quickly.
*) I once started implementing a perl to python converter. Damn lot of work.
It is not only in XML files.
> I never meant this. The DSL should be simply as it could be. This will
> allow using existing parsers and tree-builders, no mater which language
> the client is implemented in. These should exist in every language which
> is capable of implementing a Tryton client. This would not require
> implementing a python VM in another language.
Some background on what I meant, including implementations for different
languages:
http://en.wikipedia.org/wiki/Parsing_expression_grammar
>> But I understood that you prefer a single language. Unfortunately this
>> will force users to change the XML-Files.
>
> It is not only in XML files.
Even worse :-( We need to provide some conversation tool :-(
If we want to remove the python string for the client. I think it is better to
remove it also from the server side.
Because indeed those strings are already a issue in some cases see:
http://hg.tryton.org/hgwebdir.cgi/modules/project/file/5e2baff83a4b/work.py#l26
I propose to replace it by a syntax composed of python object like this:
Tuple(If(In(Eval('company'), Eval('context')), '=', '!='), Eval('id'),
Get(Eval('context'), 'company'))
The advantages are:
- they can be manipulated by Python on the server-side because it is Python
Object
- we can implemented a json function to dump it on the fly into a json string
like:
{'__class__': 'Tuple', 'v': {'__class__': 'If', 'v': {...}}}
And the client can inteprete it when loading the json.
- they are not soo much verbose compare to Python statement
- we good implement specific function for common pattern
Disavantages are:
- we need to check all the strings in XML (domain and context of
ir.action.act_window) and Fields (domain and states). But any way, they was
not so easy to extend as I showed above.
- it will increase a little bit the size of fields_view_get result.
--
Cédric Krier
B2CK SPRL
Rue de Rotterdam, 4
4000 Liège
Belgium
Tel: +32 472 54 46 59
Email: cedric...@b2ck.com
Jabber: cedric...@b2ck.com
> I propose to replace it by a syntax composed of python object like this:
+0.5
> Disavantages are:
> - we need to check all the strings in XML (domain and context of
> ir.action.act_window) and Fields (domain and states). But any way, they was
> not so easy to extend as I showed above.
I'm afraid we need some tool here to help user upgrading the database to
the new syntax.
There is no need to update the database as it is data that is in the code or
in XML.
> There is no need to update the database as it is data that is in the code or
> in XML.
But the data from the XML is stored in the database, isn't it? So
updating the XML files may not be enough, since user can have changed
this information directly through the client.
Or am I missing something?
The database will be updated with the data from XML and the user can not
change data that comes from XML (except for some specific cases).
> The database will be updated with the data from XML and the user can not
> change data that comes from XML (except for some specific cases).
Couldn't the the user have added his own window actions without any XML
file?
Perhaps (but highly improbable). So he should update it himself.
I do not think, this is a good idea. Tryton offers the way to configure
it via the client (thus without XML) so we customer should get some
migration help. Everything else would be unprofessional.
And such a tool would help up migrating the XML files, too. Thus the
main work has to be done anyway.
People are not required to upgrade if they don't want to do some work.
Don't forget, it is *free software*! We said that we will migrate automaticaly
official module, not every custom stuffs.
>
> And such a tool would help up migrating the XML files, too. Thus the
> main work has to be done anyway.
>
I will not spend my time on writing this tool for free. So if someone does it,
it will be used.
Here is a first draft of the implementation:
http://codereview.appspot.com/183067
> People are not required to upgrade if they don't want to do some work.
> Don't forget, it is *free software*! We said that we will migrate automaticaly
> official module, not every custom stuffs.
Experience shows that those open source projects are most successful
(=gain momentum, survive) which profide migration tools to users. System
integrators are users, too.
As there is any comments. We will continue this way.
> As there is any comments. We will continue this way.
Any reason why you are not using positional arguments? This would ease
implementation. I've put an implementation at http://dpaste.org/EOYw/
But I'm still unsure what is your aim? Should developers write this into
their files? *brrrrr* This is unmaintainable!
Why not.
>
> But I'm still unsure what is your aim? Should developers write this into
> their files? *brrrrr* This is unmaintainable!
Of course. It is not more unmaintainable than the current strings. And I think
it is more customizable and extendable.
But having one more depth in the dictionary will generate a bigger string.
I will work on the staticmethod eval on each object and submit a new patch.
Two other approaches (based on http://dpaste.org/EOYw/):
a) Use a dict with only one argument (example will result in 222 chars)::
class PYSON(object):
[...]
def pyson(self):
return {
'__class__': [self.__class__.__name__,] + self._args
}
class PYSONDecoder(json.JSONDecoder):
[...]
def object_hook(self, dct):
if '__class__' in dct:
args = dct['__class__']
klass = globals().get(args[0])
return klass.eval(self.__context, *args[1:])
return dct
["id", {"__class__": ["If", {"__class__": ["Not", {"__class__": ["In",
{"__class__": ["Eval", "context", {}]}, "company", null]}]}, "=",
"!="]}, {"__class__": ["Get", {"__class__": ["Eval", "contex1", {}]},
"company", 0]}]
b) Use only lists (example will result in 132 chars)::
class PYSON(object):
[...]
def pyson(self):
return [self.__class__.__name__,] + self._args
class PYSONDecoder(json.JSONDecoder):
[...]
def object_hook(self, args):
klass = globals().get(args[0], None)
if klass:
return klass.eval(self.__context, *args[1:])
return dct
["id", ["If", ["Not", ["In", ["Eval", "context", {}], "company",
null]], "=", "!="], ["Get", ["Eval", "contex1", {}], "company", 0]]
--
Goebel Consult, CISSP
Spezialist f�r IT-Sicherheit in komplexen Umgebungen
http://www.goebel-consult.de
> Of course. It is not more unmaintainable than the current strings. And I
> think
> it is more customizable and extendable.
Sorry, I'm still missing something here. How will this
<field name="domain">[('fiscalyear.company.id', '=',
context.get('company', False))]</field>
be written in future? Like this::
<field name="domain">
[('Get(Get('fiscalyear', 'company'), 'id'),
'=',
'Get(Eval('contex1', {}), 'company', 0)'
)]</field>
?
--
Goebel Consult, CISSP
Spezialist f�r IT-Sicherheit in komplexen Umgebungen
http://www.goebel-consult.de
The issue is that in json, it is a common practice to use __class__ to extend
json to handle new class. So it is expected to have a string as value, like I
do in the json-rpc patch.
>
>
> b) Use only lists (example will result in 132 chars)::
> class PYSON(object):
> [...]
> def pyson(self):
> return [self.__class__.__name__,] + self._args
> class PYSONDecoder(json.JSONDecoder):
> [...]
> def object_hook(self, args):
> klass = globals().get(args[0], None)
> if klass:
> return klass.eval(self.__context, *args[1:])
> return dct
>
> ["id", ["If", ["Not", ["In", ["Eval", "context", {}], "company",
> null]], "=", "!="], ["Get", ["Eval", "contex1", {}], "company", 0]]
>
The issue for this one is what about a list that will have one of this key
words ("Eval", "If" etc).
For now it is:
<field name="domain">
["('fiscalyear.company.id', '=', context.get('company', False))"]
</field>
>
> be written in future? Like this::
>
> <field name="domain">
> [('Get(Get('fiscalyear', 'company'), 'id'),
> '=',
> 'Get(Eval('contex1', {}), 'company', 0)'
> )]</field>
<field name="domain">
[('fiscalyear.company.id', '=', Get(Eval('context', {}), 'company', 0))]
</field>
> <field name="domain">
> [('fiscalyear.company.id', '=', Get(Eval('context', {}), 'company', 0))]
> </field>
Buh, am I glad! I was afraid, users should write json-Syntax here. This
one is okay and quite readable. :-)
So I'm only missing one link: Where is the code interpreting / evaluating
this expressions?
It will be interpreting in the server side in modelview and in
ir.action.act_window.
It will be evaluated on the client side in many places and on the server side
in the modelstorage validation part.
> The issue is that in json, it is a common practice to use __class__ to
> extend
> json to handle new class. So it is expected to have a string as value,
IC.
>> ["id", ["If", ["Not", ["In", ["Eval", "context", {}], "company",
>> null]], "=", "!="], ["Get", ["Eval", "contex1", {}], "company", 0]]
>>
>
> The issue for this one is what about a list that will have one of this key
> words ("Eval", "If" etc).
Should not be a problem:
* The first element in the list is _always_ the function/class-name.
* All (remaining) elements are arguments which are either
- terminal symbols ('company', 0, 'Eval') or
- function calls (this is lists.
* If an argument is a list, it will always be another function call --
since we doe not know the datatype list. (If we are intruding lists, this
will become a new function/class "List".)
Examples:
* ["Eval", "If"] -> Eval("If")
argument is a terminal symbol
* ["Eval", ["If", "cond", "yes", "no"]] -> Eval(If("cond", "yes", "no"))
argument is a list and thus a nested call
Testcode can be found at http://dpaste.org/oxVz/
>> So I'm only missing one link: Where is the code interpreting /
>> evaluating
>> this expressions?
>
> It will be interpreting in the server side in modelview and in
> ir.action.act_window.
>
> It will be evaluated on the client side in many places and on the server
> side
> in the modelstorage validation part.
Yes, but where is the code which actually does the interpretation? Is it
still to be written or did I overlook it?
No. We use json notation where a list is a list and we add some hook to handle
some kind of list differently.
The goals is to able to define dynamic domains in Tryton like this:
[('name', 'in', ['Foo', 'Bar'])]
where the list will be dump in json with the pyson hook.
So what about this domain:
[('name', 'in', ['Eval', 'Bar'])]
This will not work in your implementation as the list will be considered as an
eval statement instead of a list object.
For now, it is simple eval. But it will be replace with the pyson eval once
pyson will be ready.
> No. We use json notation where a list is a list and we add some hook
> to handle some kind of list differently. The goals is to able to
Okay, thus by approach does not work. Never mind.
--
Schönen Gruß - Regards
Hartmut Goebel
Dipl.-Informatiker (univ.), CISSP, CSSLP
Goebel Consult
Spezialist für IT-Sicherheit in komplexen Umgebungen
http://www.goebel-consult.de
Monatliche Kolumne: http://www.cissp-gefluester.de/
- Server: http://codereview.appspot.com/183067
- Client: http://codereview.appspot.com/186196
I have also modified all the modules, I will upload on Rietveld if someone
really will review it.
It requires a simple -u all for the database. Custom modules must be updated
to use PySON instead of evaluated strings. There is some check in the Tryton
startup that will raise exception if there is bad argument on fields and also
check on view when updating/installing modules.
There is one case that was not possible to verify, it is in Wizard when it
returns a fake ir.action.act_window, it must return pyson_* values like in
here:
http://hg.tryton.org/hgwebdir.cgi/trytond/diff/403ba0f2a1e5/trytond/ir/translation.py
As it is a major changeset, it will be good that everybody re-test all Tryton
to find missing issues.
Thanks,