diff mbox series

[ovs-dev] Python: Make Row's __getattr__ less error prone

Message ID 20181005153107.21667-1-lucasagomes@gmail.com
State Accepted
Headers show
Series [ovs-dev] Python: Make Row's __getattr__ less error prone | expand

Commit Message

Lucas Alvares Gomes Oct. 5, 2018, 3:31 p.m. UTC
From: Lucas Alvares Gomes <lucasagomes@gmail.com>

Calling getattr() on a Row object after invoking delkey() with a value
that does not exist in the object will cause getattr() to fail with a
KeyError error. For example:

Oct 05 14:59:28 neutron-server[28435]:   File
"/usr/local/lib/python2.7/dist-packages/ovsdbapp/backend/ovs_idl/connection.py",
line 122, in run
Oct 05 14:59:28 neutron-server[28435]:
txn.results.put(txn.do_commit())
Oct 05 14:59:28 neutron-server[28435]:   File
"/usr/local/lib/python2.7/dist-packages/ovsdbapp/backend/ovs_idl/transaction.py",
line 86, in do_commit
Oct 05 14:59:28 neutron-server[28435]:     command.run_idl(txn)
Oct 05 14:59:28 neutron-server[28435]:   File
"/usr/local/lib/python2.7/dist-packages/ovsdbapp/backend/ovs_idl/command.py",
line 299, in run_idl
Oct 05 14:59:28 neutron-server[28435]:     if
isinstance(getattr(record, self.column), dict):
Oct 05 14:59:28 neutron-server[28435]:   File
"/usr/local/lib/python2.7/dist-packages/ovs/db/idl.py", line 843, in
__getattr__
Oct 05 14:59:28 neutron-server[28435]:     del dmap[key]
Oct 05 14:59:28 neutron-server[28435]: KeyError: 'bogusvalue'

This patch is replacing the "del dmap[key]" instruction with a
"dmap.pop(key, None)" instruction instead because a pop() (with a
default value) will not raise an exception in case the key does not
exist in the object in the first place, it will just ignore it.

Signed-Off-By: Lucas Alvares Gomes <lucasagomes@gmail.com>
---
 python/ovs/db/idl.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ben Pfaff Oct. 6, 2018, 12:43 a.m. UTC | #1
On Fri, Oct 05, 2018 at 04:31:07PM +0100, lucasagomes@gmail.com wrote:
> From: Lucas Alvares Gomes <lucasagomes@gmail.com>
> 
> Calling getattr() on a Row object after invoking delkey() with a value
> that does not exist in the object will cause getattr() to fail with a
> KeyError error. For example:
> 
> Oct 05 14:59:28 neutron-server[28435]:   File
> "/usr/local/lib/python2.7/dist-packages/ovsdbapp/backend/ovs_idl/connection.py",
> line 122, in run
> Oct 05 14:59:28 neutron-server[28435]:
> txn.results.put(txn.do_commit())
> Oct 05 14:59:28 neutron-server[28435]:   File
> "/usr/local/lib/python2.7/dist-packages/ovsdbapp/backend/ovs_idl/transaction.py",
> line 86, in do_commit
> Oct 05 14:59:28 neutron-server[28435]:     command.run_idl(txn)
> Oct 05 14:59:28 neutron-server[28435]:   File
> "/usr/local/lib/python2.7/dist-packages/ovsdbapp/backend/ovs_idl/command.py",
> line 299, in run_idl
> Oct 05 14:59:28 neutron-server[28435]:     if
> isinstance(getattr(record, self.column), dict):
> Oct 05 14:59:28 neutron-server[28435]:   File
> "/usr/local/lib/python2.7/dist-packages/ovs/db/idl.py", line 843, in
> __getattr__
> Oct 05 14:59:28 neutron-server[28435]:     del dmap[key]
> Oct 05 14:59:28 neutron-server[28435]: KeyError: 'bogusvalue'
> 
> This patch is replacing the "del dmap[key]" instruction with a
> "dmap.pop(key, None)" instruction instead because a pop() (with a
> default value) will not raise an exception in case the key does not
> exist in the object in the first place, it will just ignore it.
> 
> Signed-Off-By: Lucas Alvares Gomes <lucasagomes@gmail.com>

Thanks, applied to master and backported as far as it would go.
diff mbox series

Patch

diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
index 03110a76f..250e89756 100644
--- a/python/ovs/db/idl.py
+++ b/python/ovs/db/idl.py
@@ -855,7 +855,7 @@  class Row(object):
                     if removes is not None:
                         for key in removes:
                             if key not in (inserts or {}):
-                                del dmap[key]
+                                dmap.pop(key, None)
                     datum = data.Datum.from_python(column.type, dmap,
                                                    _row_to_uuid)
             else: