On Wed, May 30, 2012 at 11:02:50AM -0400, Saggi Mizrahi wrote:
If you don't want to add it in a parameter then you already suspect that you are doing something wrong. Using a singleton instead of passing a parameter doesn't make the dependency not there. It's just obscures it.
Right. Passing around an internal 'singleton' (whether wrapped or not) is not the correct course.
I might not fully understand what you want to do but I think what you want is to have MOM expect a certain interface. Then have an adapter class bridging the two interfaces. The pass the wrapped CIF to MOM.
It seems that your suggestion obscures the dependency at least as badly as a singleton interface does. Today clientIF is a singleton class in all but the name. It is basically a global container that stores global objects (vmContainer, locks, etc) and global threads (irs, hostStatsThread, etc). The fact that a reference gets passed around doesn't really make it any cleaner. We can all agree that clientIF (in its current form) has overstayed its welcome. Mark and Xu He Jie have some ideas on how to remove it and plan to tackle that work. That being said, MOM is an important feature for oVirt 3.1. Do we really want it to drop out of the release in order to focus on clientIF cleanups (which certainly will not make 3.1)?
Using a singleton simply means that API objects can be instantiated without the caller needing to be aware of clientIF at all. In my opinion, this is a cleanup because we are free to refactor clientIF without affecting the bindings or MOM one bit.
----- Original Message -----
From: "Mark Wu" wudxw@linux.vnet.ibm.com To: vdsm-devel@lists.fedorahosted.org Cc: "Dan Kenigsberg" danken@redhat.com, "Saggi Mizrahi" smizrahi@redhat.com, "Adam Litke" agl@us.ibm.com, "Ryan Harper" ryanh@us.ibm.com Sent: Wednesday, May 30, 2012 10:49:29 AM Subject: VDSM API/clientIF instance design issue
Hi Guys,
Recently, I has been working on integrate MOM into VDSM. MOM needs to use VDSM API to interact with it. But currently, it requires the instance of clientIF to use vdsm API. Passing clientIF to MOM is not a good choice since it's a vdsm internal object. So I try to remove the parameter 'cif' from the interface definition and change to access the globally unique clientIF instance in API.py.
To get the instance of clientIF, I add a decorator to clientIF to change it into singleton. Actually, clientIF has been working as a global single instance already. We just don't have an interface to get it and so passing it as parameter instead. I think using singleton to get the instance of clientIF is more clean.
Dan and Saggi already gave some comments in http://gerrit.ovirt.org/#change,4839 Thanks for the reviewing! But I think we need more discussion on it, so I post it here because gerrit is not the appropriate to discuss a design issue.
Thanks ! Mark.