Hi Owen, There are quite a few emails in this thread already, but there are points in each of them I would like to address. Up front let me say I hear and recognize your frustration. If you’ve felt ignored, I apologize. I think you’ve turned around a lot of patches, PR comments, and emails faster then I’ve been able to look at them all. It’s completely been “I haven’t gotten there yet” and none of “I’m ignoring you”. > On Jul 9, 2015, at 3:08 AM, Owen Synge <osynge@xxxxxxxx> wrote: > > Dear all, > > The facade pattern (or façade pattern) is a software design pattern > commonly used with object-oriented programming. The name is by analogy > to an architectural facade. (wikipedia) > > I am frustrated with the desire to "standardise" on the one bad practice > implementations of the facade pattern in python that is used in > ceph-deploy even in ceph. > > The current "standard" of selectively calling modules with functions has > a series of complexities to it. > > Ceph example: > > https://github.com/ceph/ceph/tree/master/src/ceph-detect-init/ceph_detect_init > > ceph-deploy example: > > https://github.com/ceph/ceph-deploy/tree/master/ceph_deploy/hosts > > SO I guess _some_ of you dont immediately see why this is VERY bad > practice, and wonder why this makes me feel like the orange in this story: > > http://www.dailymail.co.uk/news/article-2540670/The-perfect-half-time-oranges-five-football-matches-Farmers-create-pentagon-shaped-fruit.html > > when I try to code with ceph-deploy in particular. I’ve never heard the facade name attached to it. For our modest goals with ceph-deploy we have found it be quite flexilble, as it allows us to do have distro specific code only where it is needed, and to pull all common code into common places, all behind a consistent abstracted interface. I think this actually leads to less code duplication, but there are plenty of places in ceph-deploy where this is not the case and the code could be cleaned up to bring more code into common places. Let me also say that specific to one of the comments I made in your PR, when I brought up this facade idea, I was just talking out loud. The comment I was going to leave today (and still will) is that my point there was unrelated to your PR. It sent us off in a wrong direction, and was not relevant to the code under discussion. It was more a note to myself that this was something that was still on our plate, but it did not/does not need to change what you were looking at specifically. > > The ceph/ceph-deploy way of implementing a facade in python causes this > list the problems: > > 1) facade cannot be instantiated twice. > > 2) facade requires code layout inflexibility. > > 3) facade implementation can have side effects when implementation is > changed. > > And probably others I have not thought of. > > In consequence from points above. > > From (1) > > (1A) no concurrency, so you cant configure more than one ceph-node at a > time. > (1A) You have to close one facade to start anouther, eg in ceph-deploy > you have to close each connection before connecting to the next server > so making it slow to use as all state has to be gathered. concurrency has come up before in ceph-deploy. It has been our explicit goal to make ceph-deploy as simple and *clear* as possible for users, with one of the main purposes to be extremely verbose and essentially *teach* a user how to deploy a Ceph cluster. That’s why it prints everything it does by default, shows every remote command, and prints the output back in order. Concurrency would muddy those waters, though we do all want things to go faster. It is not necessarily the facade pattern that is the limitation there — it is the implementation within ceph-deploy. We simply do a “for host is hostnames…” loop everywhere — it doesn’t matter what we are using underneath, we are doing one SSH connection at a time. Furthermore, I don’t think it’s the facade paradigm that would limit you to one host at a time at all. It’s one host module (facade) instantiated per host. You could do many of these at once — I don’t see any reason why you couldn’t. I’ve never tried it out, and I don’t know if python-remoto handles concurrency, but I don’t think the facade paradigm prevents it. > > From (2) > > (2A) No nesting of facades without code duplication. EG reuse of systemd > support between distributions. > (2B) inflexible / complex include paths for shared code between facade > implementations. > I disagree here. There are plenty of places to put code that is common and does not have to be duplicated. I definitely don’t want code in each host module that knows how to deal with systemd. The code that does have to be in each host module is how to detect/assign what init system is being used, and how to call into the appropriate (common) module to do what it needs. As for complex include paths, I’m not sure I have much of an argument here other than to say that all frameworks have their conventions. I don’t see what we are doing as anything different than what is enforced/recommended by many frameworks/scaffolds popular in Web app and MVC paradigms like RoR, Pyramid, Django, etc. It’s just convention to learn. > From (3) > > (3A) Since all variables in a facade implementation are global, but > isolated to an implementation, we cannot have variables in the > implementation, any cross function variables that are not passed as > parameters or return values will return will lead to side effects that > are difficult to test. I admit I don’t follow this one completely. > > So this set of general points will have complex side effects that make > you feel like an the pictured orange when developing that are related to > wear the facade is implemented. > > About this point you will say well its open source so fix it ? > > My answer to this is that when I try to do this, as in this patch: > > https://github.com/osynge/ceph-deploy/commit/b82f89f35b27814ed4aba1082efd134c24ecf21f > > More than once, seem to suggest I should use the much more complex multi > file implementation of a façade. > > The only advantages of implementing façades in the "standard" ceph / > ceph-deploy way that I can see is: > > (I) how ceph-deploy has always done this way. > (II) It allows you to continue using nearly no custom objects in python. > (III) We like our oranges in funny shapes. Again, I have more to say here in the actual PR, but do give me some time. I can tell you it will be at least 1.5 hrs from now before I will get to go read it all again. In short, yes I would like to continue to use what we have, but I think the classes you’ve created to handle systemd and sysv can fit into that very nicely. > > It seems to me the current implementation could have been created due to > the misunderstanding that modules are like objects, when intact they are > like classes. Issues (1) and (3) can be solved simply by importing > methods as objects rather than classes, but this does nothing to solve > the bigger issue (2) which is more serious, but its a simple step > forward that might very simple to patch, but their is little point in a > POC unless people agree that issues (1) (2) and (3) are serious. > > Please can we not spread this boat anchor* implementation of a facade, > further around the code base, and ideally migrate away from this bad > practice, and help us all feel like happy round oranges. > John touched on this a bit already, about the real question here being about ultimate goals and intentions. I’ll come back to this in another email, but I have a couple of other scheduled events to attend to first. Again, do please give me the time to reply properly. > Best regards > > Owen > > > boat anchor > > https://en.wikipedia.org/wiki/Boat_anchor_%28metaphor%29 -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
