----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/723/ -----------------------------------------------------------
Review request for OpenLMI Developers.
Repository: openlmi-providers
Description -------
Here's a new registration script. It's a merge of the existing openlmi-mof-register and openlmi-register pegasus and should work as drop-in replacement for the openlmi-mof-register. The only slight difference is that the Pegasus interop namespace is not hard coded into the Pegasus registration calls but now there's an attempt to detect the right one. And the whole thing is now written in (sort of) Python.
This is only the first step in the registration rewrite: Python should make it easier for manipulating a "side database" that would keep the registration data independently of the CIMOMs and eventually allow for re-registration when a new CIMOM is installed or upgraded. This will also make possible for the providers package not to depend on a CIMOM. I will prepare some design sketches of the registration process I envision to gather feedback and ideas: doing this correctly and robustly enough is going to be rather difficult.
Diffs -----
openlmi-mof-register.py PRE-CREATION
Diff: http://reviewboard-openlmi.rhcloud.com/r/723/diff/
Testing -------
Thanks,
Tomas Smetana
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/723/#review917 -----------------------------------------------------------
openlmi-mof-register.py http://reviewboard-openlmi.rhcloud.com/r/723/#comment487
I think that this shouldn't be here.
openlmi-mof-register.py http://reviewboard-openlmi.rhcloud.com/r/723/#comment488
I think that this shouldn't be here.
openlmi-mof-register.py http://reviewboard-openlmi.rhcloud.com/r/723/#comment489
At this point, I think it would be a good idea to exit() with an error message (maybe a broken installation?). Unless Pegasus can have no "root/interop" or "root/PG_InterOp" and still work properly.
- Jan Synacek
On Aug. 15, 2013, 3:50 p.m., Tomas Smetana wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/723/
(Updated Aug. 15, 2013, 3:50 p.m.)
Review request for OpenLMI Developers.
Repository: openlmi-providers
Description
Here's a new registration script. It's a merge of the existing openlmi-mof-register and openlmi-register pegasus and should work as drop-in replacement for the openlmi-mof-register. The only slight difference is that the Pegasus interop namespace is not hard coded into the Pegasus registration calls but now there's an attempt to detect the right one. And the whole thing is now written in (sort of) Python.
This is only the first step in the registration rewrite: Python should make it easier for manipulating a "side database" that would keep the registration data independently of the CIMOMs and eventually allow for re-registration when a new CIMOM is installed or upgraded. This will also make possible for the providers package not to depend on a CIMOM. I will prepare some design sketches of the registration process I envision to gather feedback and ideas: doing this correctly and robustly enough is going to be rather difficult.
Diffs
openlmi-mof-register.py PRE-CREATION
Diff: http://reviewboard-openlmi.rhcloud.com/r/723/diff/
Testing
Thanks,
Tomas Smetana
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/723/#review918 -----------------------------------------------------------
Ship it!
openlmi-mof-register.py http://reviewboard-openlmi.rhcloud.com/r/723/#comment490
According to PEP8: "version == None" is wrong, use "version is None".
http://www.python.org/dev/peps/pep-0008/#programming-recommendations
Looks good. Feel free to ship it now or add the database first.
- Radek Novacek
On Aug. 15, 2013, 3:50 p.m., Tomas Smetana wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/723/
(Updated Aug. 15, 2013, 3:50 p.m.)
Review request for OpenLMI Developers.
Repository: openlmi-providers
Description
Here's a new registration script. It's a merge of the existing openlmi-mof-register and openlmi-register pegasus and should work as drop-in replacement for the openlmi-mof-register. The only slight difference is that the Pegasus interop namespace is not hard coded into the Pegasus registration calls but now there's an attempt to detect the right one. And the whole thing is now written in (sort of) Python.
This is only the first step in the registration rewrite: Python should make it easier for manipulating a "side database" that would keep the registration data independently of the CIMOMs and eventually allow for re-registration when a new CIMOM is installed or upgraded. This will also make possible for the providers package not to depend on a CIMOM. I will prepare some design sketches of the registration process I envision to gather feedback and ideas: doing this correctly and robustly enough is going to be rather difficult.
Diffs
openlmi-mof-register.py PRE-CREATION
Diff: http://reviewboard-openlmi.rhcloud.com/r/723/diff/
Testing
Thanks,
Tomas Smetana
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/723/#review926 -----------------------------------------------------------
openlmi-mof-register.py http://reviewboard-openlmi.rhcloud.com/r/723/#comment510
This could be turned to docopt parseable usage string. It's pity we don't want to depend on it :(.
Anyway it would be nice to use some smart argument parser, that would do the hard work of checking for us. For example argparse. But this could be done later.
openlmi-mof-register.py http://reviewboard-openlmi.rhcloud.com/r/723/#comment506
I'd probably move the body of conditions to separate functions (unregister_sfcbd(), unregister_pegasus()).
openlmi-mof-register.py http://reviewboard-openlmi.rhcloud.com/r/723/#comment509
Since ``devnull`` is handy on multiple places like this, I think it would make sense to make it a global variable.
Or make an override for ``subprocess.call()`` like this:
def call(*args, **kwargs): with open("/dev/null", "w") as devnull: kwargs['stdout'] = devnull kwargs['stderr'] = devnull return subprocess.call(*args, **kwargs)
usage: ret = call(["/usr/sbin/cimserver", "--status"])
openlmi-mof-register.py http://reviewboard-openlmi.rhcloud.com/r/723/#comment508
Please use: with open("regfile", "w") as regfile: # guarded code with ``regfile`` available # no explicit ``close()`` call needed
It's more readable and robust. It applies to other open() calls as well.
- Michal Minar
On Aug. 15, 2013, 1:50 p.m., Tomas Smetana wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/723/
(Updated Aug. 15, 2013, 1:50 p.m.)
Review request for OpenLMI Developers.
Repository: openlmi-providers
Description
Here's a new registration script. It's a merge of the existing openlmi-mof-register and openlmi-register pegasus and should work as drop-in replacement for the openlmi-mof-register. The only slight difference is that the Pegasus interop namespace is not hard coded into the Pegasus registration calls but now there's an attempt to detect the right one. And the whole thing is now written in (sort of) Python.
This is only the first step in the registration rewrite: Python should make it easier for manipulating a "side database" that would keep the registration data independently of the CIMOMs and eventually allow for re-registration when a new CIMOM is installed or upgraded. This will also make possible for the providers package not to depend on a CIMOM. I will prepare some design sketches of the registration process I envision to gather feedback and ideas: doing this correctly and robustly enough is going to be rather difficult.
Diffs
openlmi-mof-register.py PRE-CREATION
Diff: http://reviewboard-openlmi.rhcloud.com/r/723/diff/
Testing
Thanks,
Tomas Smetana
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/723/#review927 -----------------------------------------------------------
Thanks everyone for the comments.
openlmi-mof-register.py http://reviewboard-openlmi.rhcloud.com/r/723/#comment518
It depends on the "depth" of the dependency: This is an utility script and I would like to avoid it to drag in too many dependencies of its own. ([Disclaimer] I have no idea what docopt is: I tend to write a C code no matter what language I use...)
openlmi-mof-register.py http://reviewboard-openlmi.rhcloud.com/r/723/#comment511
There is a commented-out subprocess-call that makes use of the devnull. It's mimicking the behaviour of the original openlmi-mof-register script bu I found it useful to see the stderr of the called program as well. You're right that this part will need some ipmrovements: Michal has suggested a way to fix all the /dev/null redirected subprocess invocations.
openlmi-mof-register.py http://reviewboard-openlmi.rhcloud.com/r/723/#comment512
dtto
openlmi-mof-register.py http://reviewboard-openlmi.rhcloud.com/r/723/#comment513
Right. I will fix this.
openlmi-mof-register.py http://reviewboard-openlmi.rhcloud.com/r/723/#comment514
I don't disagree. However for now I would prefer to have the script easily comparable to the original one before I'm reasonably sure I didn't introduce any new bugs.
openlmi-mof-register.py http://reviewboard-openlmi.rhcloud.com/r/723/#comment515
Looks nice. Thanks. Will fix that.
openlmi-mof-register.py http://reviewboard-openlmi.rhcloud.com/r/723/#comment517
Will do.
openlmi-mof-register.py http://reviewboard-openlmi.rhcloud.com/r/723/#comment516
Fixed. Will post a new version later.
- Tomas Smetana
On Aug. 15, 2013, 3:50 p.m., Tomas Smetana wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/723/
(Updated Aug. 15, 2013, 3:50 p.m.)
Review request for OpenLMI Developers.
Repository: openlmi-providers
Description
Here's a new registration script. It's a merge of the existing openlmi-mof-register and openlmi-register pegasus and should work as drop-in replacement for the openlmi-mof-register. The only slight difference is that the Pegasus interop namespace is not hard coded into the Pegasus registration calls but now there's an attempt to detect the right one. And the whole thing is now written in (sort of) Python.
This is only the first step in the registration rewrite: Python should make it easier for manipulating a "side database" that would keep the registration data independently of the CIMOMs and eventually allow for re-registration when a new CIMOM is installed or upgraded. This will also make possible for the providers package not to depend on a CIMOM. I will prepare some design sketches of the registration process I envision to gather feedback and ideas: doing this correctly and robustly enough is going to be rather difficult.
Diffs
openlmi-mof-register.py PRE-CREATION
Diff: http://reviewboard-openlmi.rhcloud.com/r/723/diff/
Testing
Thanks,
Tomas Smetana
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/723/ -----------------------------------------------------------
(Updated Oct. 3, 2013, 10:42 a.m.)
Status ------
This change has been discarded.
Review request for OpenLMI Developers.
Repository: openlmi-providers
Description -------
Here's a new registration script. It's a merge of the existing openlmi-mof-register and openlmi-register pegasus and should work as drop-in replacement for the openlmi-mof-register. The only slight difference is that the Pegasus interop namespace is not hard coded into the Pegasus registration calls but now there's an attempt to detect the right one. And the whole thing is now written in (sort of) Python.
This is only the first step in the registration rewrite: Python should make it easier for manipulating a "side database" that would keep the registration data independently of the CIMOMs and eventually allow for re-registration when a new CIMOM is installed or upgraded. This will also make possible for the providers package not to depend on a CIMOM. I will prepare some design sketches of the registration process I envision to gather feedback and ideas: doing this correctly and robustly enough is going to be rather difficult.
Diffs -----
openlmi-mof-register.py PRE-CREATION
Diff: http://reviewboard-openlmi.rhcloud.com/r/723/diff/
Testing -------
Thanks,
Tomas Smetana
openlmi-reviews@lists.fedorahosted.org