Update patch set 1

Patch Set 1: I would prefer that you didn't merge this

(1 inline comment)

One comment inline to clean things up a bit, and handle the "no such extension" case, for example if the command is "doesnotexist.do_something".

I'd also like to see a test for the "no such extension" case.

Otherwise looks good, thanks for doing this!

Patch-set: 1
Label: Code-Review=-1
Label: Workflow=0
This commit is contained in:
Gerrit User 10343 2014-03-27 00:59:05 +00:00 committed by Gerrit Code Review
parent de8e6a93f7
commit 075225b70f
1 changed files with 21 additions and 0 deletions

View File

@ -0,0 +1,21 @@
{
"comments": [
{
"key": {
"uuid": "AAAAWH/+aFo\u003d",
"filename": "ironic_python_agent/agent.py",
"patchSetId": 1
},
"lineNbr": 180,
"author": {
"id": 10343
},
"writtenOn": "2014-03-27T00:59:05Z",
"side": 1,
"message": "Since there will only ever be one extension for a given name, I think this could be simplified by just doing something like:\n\n try:\n ext \u003d self.ext_mgr[extension_part]\n except KeyError:\n # raise some error about extension not found\n func \u003d getattr(ext.obj, command_part)\n result \u003d func(**kwargs)\n\nThis also removes the need for the `_command_execute` function above, and the FIXME below.",
"revId": "3e2a712343c9bdcb100902bb1b4f41b234ec2c69",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
}
]
}