[6.0] Inheriting _constraints "list index out of range" if original len(_constraint ) is bigger than the new one

[Nhomar - Vauxoo]

branch: 6.0
addons : 4677
server: 3452
My module example:
class stock_move_constraint(osv.osv):
    _inherit = 'stock.move'
    _columns = {}
    def _check_import_info(self, cr, uid, ids, context=None):
        print "I checked"
        return True
    _constraints = [
        (_check_import_info,'You must assign a track lot with import information for this product',['tracking_id'])],
stock_move_constraint()

When I try to update my module ($python openerp-server.py -u mymodule -d mydb) this traceback is received.

Traceback (most recent call last):
  File "./openerp-server.py", line 121, in <module>
    db,pool = pooler.get_db_and_pool(dbname, update_module=tools.config['init'] or tools.config['update'], pooljobs=False)
  File "/home/nhomar/CD_curso/linux_and_sources/server/bin/pooler.py", line 39, in get_db_and_pool
    addons.load_modules(db, force_demo, status, update_module)
  File "/home/nhomar/CD_curso/linux_and_sources/server/bin/addons/__init__.py", line 883, in load_modules
    processed_modules.extend(load_module_graph(cr, graph, status, report=report, skip_modules=processed_modules))
  File "/home/nhomar/CD_curso/linux_and_sources/server/bin/addons/__init__.py", line 717, in load_module_graph
    modules = pool.instanciate(package.name, cr)
  File "/home/nhomar/CD_curso/linux_and_sources/server/bin/osv/osv.py", line 258, in instanciate
    res.append(klass.createInstance(self, module, cr))
  File "/home/nhomar/CD_curso/linux_and_sources/server/bin/osv/osv.py", line 354, in createInstance
    if new[c2][2]==c[2] and (new[c2][0] == c[0] \
IndexError: list index out of range

I printed variables of this method (check patch it is corrected) and some kind of bad logic is used, the server is trying to check as a list an integer....

Original problem
~~~~LINE 348 on osv.py

                        if s=='_constraints':
                            for c in cls.__dict__.get(s, []):
                                exist = False
                                for c2 in range(len(new)):
 ####THIS IS MY PRINT it is an integer,,,,,
#on the range(len(new)) always will return an index in c2 of the original class never, and if the original
#_constraint list in class is bigger than the new one it will broke.
                                     print c2
                                     #For _constraints, we should check field and methods as well
                                     if new[c2][2]==c[2] and (new[c2][0] == c[0] \
                                            or getattr(new[c2][0],'__name__', True) == \
                                                getattr(c[0],'__name__', False)):
                                        # If new class defines a constraint with
                                        # same function name, we let it override
                                        # the old one.
                                        new[c2] = c
                                        exist = True
                                        break
                                if not exist:
                                    new.append(c)

[Vinay Rana (OpenERP) (vra-openerp)]

Hello Nhomar,

I have checked similar issue with latest stable and trunk code but did not face any traceback while updating the exiting db.
Would you please provide me more steps to reproduce the same traceback at my end.

Thanks.

[xrg (xrg)]

On Monday 27 June 2011, you wrote:
> ** Patch added: "Patch for problem"
>
> https://bugs.launchpad.net/bugs/802352/+attachment/2182312/+files/patch_co
> nstraints.diff

Hi,
for that piece of code you mention, I should be held responsible, I wrote it.

But I can't understand your patch:
you seem to do an iteration over 'c', I don't understand why.

At line osv.py:347, there is:
   for c in cls.__dict__.get(s, []):
which should iterate over the "_constraints" list. So, 'c' should point to
just one constraint (3-item tuple)

Are you sure your constraints are in the form of:
    _constraints = [ ('foo', 'bar', 'expl'), ('foo2', 'bar2', 'expl2') ]

and NOT accidentally like
   _constraints = [ ('foo', 'bar', 'expl'), [ ('foo2', 'bar2', 'expl2'), ], ]

???

--
Say NO to spam and viruses. Stop using Microsoft Windows!

[Nhomar - Vauxoo (nhomar)]

Hello Vinay.

Try to inherit exactly the model that i putted on the description in bug,
this is a good example, the model 'stock.move' originally has 2 constraints,
i put on my inheritance one more (less than the original one), if you see
the original code, you can see that the server is itering between the
original, but the inherited is not itered it is just called, with this the
server is considering just one case....

I attach one db before install module:
#DATABASE INFO
db_password = 123
list_db = True
db_port = False
db_name = False
db_user = guadalajara
db_host = False

the branch (there are a module just presenting this issue, ) try to install
this module.

BRANCH: lp:~openerp-commiter/openobject-server/constrain_Bug_802352

Regards.

2011/6/27 Vinay Rana (openerp) <email address hidden>

> Hello Nhomar,
>
> I have checked similar issue with latest stable and trunk code but did not
> face any traceback while updating the exiting db.
> Would you please provide me more steps to reproduce the same traceback at
> my end.
>
> Thanks.
>
> ** Changed in: openobject-server
> Status: New => Incomplete
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/802352
>
> Title:
> [6.0] Inheriting _constraints "list index out of range" if original
> len(_constraint ) is bigger than the new one
>
> Status in OpenERP Server:
> Incomplete
>
> Bug description:
> branch: 6.0
> addons : 4677
> server: 3452
> My module example:
> class stock_move_constraint(osv.osv):
> _inherit = 'stock.move'
> _columns = {}
> def _check_import_info(self, cr, uid, ids, context=None):
> print "I checked"
> return True
> _constraints = [
> (_check_import_info,'You must assign a track lot with import
> information for this product',['tracking_id'])],
> stock_move_constraint()
>
> When I try to update my module ($python openerp-server.py -u mymodule
> -d mydb) this traceback is received.
>
> Traceback (most recent call last):
> File "./openerp-server.py", line 121, in <module>
> db,pool = pooler.get_db_and_pool(dbname,
> update_module=tools.config['init'] or tools.config['update'],
> pooljobs=False)
> File "/home/nhomar/CD_curso/linux_and_sources/server/bin/pooler.py",
> line 39, in get_db_and_pool
> addons.load_modules(db, force_demo, status, update_module)
> File
> "/home/nhomar/CD_curso/linux_and_sources/server/bin/addons/__init__.py",
> line 883, in load_modules
> processed_modules.extend(load_module_graph(cr, graph, status,
> report=report, skip_modules=processed_modules))
> File
> "/home/nhomar/CD_curso/linux_and_sources/server/bin/addons/__init__.py",
> line 717, in load_module_graph
> modules = pool.instanciate(package.name, cr)
> File "/home/nhomar/CD_curso/linux_and_sources/server/bin/osv/osv.py",
> line 258, in instanciate
> res.append(klass.createInstance(self, module, cr))
> File "/home/nhomar/CD_curso/linux_and_sources/server/bin/osv/osv.py",
> line 354, in createInstance
> if new[c2][2]==c[2] and (new[c2][0] == c[0] \
> IndexError: list index out of range
>
> I printed var...

[Jay Vora (Serpent Consulting Services) (jayvora)]

Hello Nhomar,

Thanks for reporting and patch as well.

However, I see that It will also be a trouble when _inheriting constraint with a combination of [OLD,NEW] comes.

I am investigating more and will post a solution with test cases.

Thanks.

[Nhomar - Vauxoo (nhomar)]

Hello xrg....

look my code.... answer in lines.

2011/6/27 xrg <email address hidden>

> On Monday 27 June 2011, you wrote:
> > ** Patch added: "Patch for problem"
> >
> >
> https://bugs.launchpad.net/bugs/802352/+attachment/2182312/+files/patch_co
> > nstraints.diff
>
> Hi,
> for that piece of code you mention, I should be held responsible, I wrote
> it.
>
> But I can't understand your patch:
> you seem to do an iteration over 'c', I don't understand why.
>
> At line osv.py:347, there is:
> for c in cls.__dict__.get(s, []):
> which should iterate over the "_constraints" list. So, 'c' should point to
> just one constraint (3-item tuple)
>
> Are you sure your constraints are in the form of:
> _constraints = [ ('foo', 'bar', 'expl'), ('foo2', 'bar2', 'expl2') ]
>
> and NOT accidentally like
> _constraints = [ ('foo', 'bar', 'expl'), [ ('foo2', 'bar2', 'expl2'), ],
> ]
>
> ???
>

in my debug, c=[('foo', 'bar', 'expl'),('foo', 'bar', 'expl'),('foo', 'bar',
'expl'),('foo', 'bar', 'expl')] all the news _constraints and you are trying
to asing just c ik you dont itter in c the "new" variable will be
new=[('old', 'const', 'raints'),[('foo', 'bar', 'expl'),('foo', 'bar',
'expl'),('foo', 'bar', 'expl'),('foo', 'bar', 'expl'),]], in this case the
module install but wxplode when the constraint is checked (i debug it ;-))

regards.

>
>
>
> --
> Say NO to spam and viruses. Stop using Microsoft Windows!
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/802352
>
> Title:
> [6.0] Inheriting _constraints "list index out of range" if original
> len(_constraint ) is bigger than the new one
>
> Status in OpenERP Server:
> In Progress
>
> Bug description:
> branch: 6.0
> addons : 4677
> server: 3452
> My module example:
> class stock_move_constraint(osv.osv):
> _inherit = 'stock.move'
> _columns = {}
> def _check_import_info(self, cr, uid, ids, context=None):
> print "I checked"
> return True
> _constraints = [
> (_check_import_info,'You must assign a track lot with import
> information for this product',['tracking_id'])],
> stock_move_constraint()
>
> When I try to update my module ($python openerp-server.py -u mymodule
> -d mydb) this traceback is received.
>
> Traceback (most recent call last):
> File "./openerp-server.py", line 121, in <module>
> db,pool = pooler.get_db_and_pool(dbname,
> update_module=tools.config['init'] or tools.config['update'],
> pooljobs=False)
> File "/home/nhomar/CD_curso/linux_and_sources/server/bin/pooler.py",
> line 39, in get_db_and_pool
> addons.load_modules(db, force_demo, status, update_module)
> File
> "/home/nhomar/CD_curso/linux_and_sources/server/bin/addons/__init__.py",
> line 883, in load_modules
> processed_modules.extend(load_module_graph(cr, graph, status,
> report=report, skip_modules=processed_modules))
> File
> "/home/nhomar/CD_curso/linux_and_sources/server/bin/addons/__init__.py",
> line 717, in load_module_graph
> modules = pool.instanciate(package.name, cr)
> File "/home/nhomar/CD_curso/linux_and_sources/server/bin/osv/osv.py",
> line 258, i...

[Nhomar - Vauxoo (nhomar)]

I try it already, just for information, i try it with my module example, and
it works, i attach before a db with several modules installed and i try all
constraints (old and new ones...)

Thanks.

2011/6/27 Jay Vora (OpenERP) <email address hidden>

> Hello Nhomar,
>
> Thanks for reporting and patch as well.
>
> However, I see that It will also be a trouble when _inheriting
> constraint with a combination of [OLD,NEW] comes.
>
> I am investigating more and will post a solution with test cases.
>
> Thanks.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/802352
>
> Title:
> [6.0] Inheriting _constraints "list index out of range" if original
> len(_constraint ) is bigger than the new one
>
> Status in OpenERP Server:
> In Progress
>
> Bug description:
> branch: 6.0
> addons : 4677
> server: 3452
> My module example:
> class stock_move_constraint(osv.osv):
> _inherit = 'stock.move'
> _columns = {}
> def _check_import_info(self, cr, uid, ids, context=None):
> print "I checked"
> return True
> _constraints = [
> (_check_import_info,'You must assign a track lot with import
> information for this product',['tracking_id'])],
> stock_move_constraint()
>
> When I try to update my module ($python openerp-server.py -u mymodule
> -d mydb) this traceback is received.
>
> Traceback (most recent call last):
> File "./openerp-server.py", line 121, in <module>
> db,pool = pooler.get_db_and_pool(dbname,
> update_module=tools.config['init'] or tools.config['update'],
> pooljobs=False)
> File "/home/nhomar/CD_curso/linux_and_sources/server/bin/pooler.py",
> line 39, in get_db_and_pool
> addons.load_modules(db, force_demo, status, update_module)
> File
> "/home/nhomar/CD_curso/linux_and_sources/server/bin/addons/__init__.py",
> line 883, in load_modules
> processed_modules.extend(load_module_graph(cr, graph, status,
> report=report, skip_modules=processed_modules))
> File
> "/home/nhomar/CD_curso/linux_and_sources/server/bin/addons/__init__.py",
> line 717, in load_module_graph
> modules = pool.instanciate(package.name, cr)
> File "/home/nhomar/CD_curso/linux_and_sources/server/bin/osv/osv.py",
> line 258, in instanciate
> res.append(klass.createInstance(self, module, cr))
> File "/home/nhomar/CD_curso/linux_and_sources/server/bin/osv/osv.py",
> line 354, in createInstance
> if new[c2][2]==c[2] and (new[c2][0] == c[0] \
> IndexError: list index out of range
>
> I printed variables of this method (check patch it is corrected) and
> some kind of bad logic is used, the server is trying to check as a
> list an integer....
>
> Original problem
> ~~~~LINE 348 on osv.py
>
> if s=='_constraints':
> for c in cls.__dict__.get(s, []):
> exist = False
> for c2 in range(len(new)):
> ####THIS IS MY PRINT it is an integer,,,,,
> #on the range(len(new)) always will return an index in c2 of the original
> class never, and if the original
> #_constraint list in class is bigger...

[Jay Vora (Serpent Consulting Services) (jayvora)]

@Panos, Nhomar is right. Even I wonder how C could have list instead of 3-item tuple(the intended one). We earlier fixed the code for other problem.

@Nhomar, Make sure you test well because I am a little sure it will add overridden constraint twice due to [new.append(n) for n in c].

Anyways, I have a patch ready and I am doing some tests. I will post the branch soon.

Keep posting.

Thanks.

[xrg (xrg)]

On Monday 27 June 2011, you wrote:
> @Panos, Nhomar is right. Even I wonder how C could have list instead of
> 3-item tuple(the intended one). We earlier fixed the code for other
> problem.
>
> @Nhomar, Make sure you test well because I am a little sure it will add
> overridden constraint twice due to [new.append(n) for n in c].
>
> Anyways, I have a patch ready and I am doing some tests. I will post the
> branch soon.
>

After a little closer examination (we wasted >1hour each on that), we
discovered that all this was caused by a _syntax error_ at your code, Nhomar.

you had:
  _constraints = [(...), ] *,*

Which in python works like:
Python 2.7.1 (r271:86832, May 22 2011, 22:07:27)
>>> a = 1,
>>> print a
(1,)
>>>

so, _constraints was fed as a tuple of list of tuples, rather than list of
tuples. The API is never expected to handle that. It *must* have just a list
of tuples.

Therefore the bug is Invalid

Lesson learned: do not mix Java into Python. Have your code lines fit in a
single screen width ! (careful with those commas, too)

[Jay Vora (Serpent Consulting Services) (jayvora)]

Panos has found out the actual issue which is a typo, he is responding here soon.

[Nhomar - Vauxoo (nhomar)]

@xrg

When somebody do his work well , this kind of thing happens.... THANKS a lot xrg you are the man ;-)

I propose one thing, this thing took from me at least 4 hours i think i can not be the only one that can have this "finger error", i propose for trunk merge the diff attached it verifiy that the typo is correct when you inherit a _constraint.

I don't know if the place for this verification is right but i propose the idea....

Thanks a Lot again GREAT WORK guys.

PD:

When you say ...
Lesson learned: do not mix Java into Python. Have your code lines fit in a single screen width ! (careful with those commas, too)

Je... I'm not Java developer, it was an "finger error", please, don't call me java developer, i feel bad enought for my error jejej!

[Vinay Rana (OpenERP) (vra-openerp)]

Hello Nhomar,

I think you can propose your improvement directly by putting a related merge proposal in to server branch.

Thanks.

[xrg (xrg)]

On Tuesday 28 June 2011, you wrote:
> I propose one thing, this thing took from me at least 4 hours i think i can
> not be the only one that can have this "finger error", i propose for trunk
> merge the diff attached it verifiy that the typo is correct when you
> inherit a _constraint.
>
> I don't know if the place for this verification is right but i propose
> the idea....

> https://bugs.launchpad.net/openobject-server/+bug/802352/+attachment/21831
> 00/+files/diff_error_controled.diff
>

Well, in principle, you have a point about this diff.

The problem is that Python is a weakly typed language, and can propagate wrong
types in parts of the code you'd never expect them. Not only they could cause
exceptions, but any kind of Pythonic (in the Monty Python sense) behavior.

The solution is to patch Python to emulate what a strong language would do.
Your patch does that, but I'd prefer to keep an even simpler (and compact)
syntax:

   for c in some_list:
      assert isinstance(c, tuple), "Why do we get %r ?" % c

Which is an one-liner, blocks exactly what we want.. We could have assertions
in many places, like the start of functions that need to ensure eg. that 'ids'
is a list.

On the other hand, even this assertion will introduce some overhead in our
code. Sometimes it's critical to save even that one line.

--
Say NO to spam and viruses. Stop using Microsoft Windows!