----- Original Message ----- > On Thu, Dec 01, 2011 at 10:13:53PM -0500, Saggi Mizrahi wrote: > > I want to suggest a different way of handling constants.py and > > config.py > > the current way is clumsy and makes making convoluted code easy. > > > > GLOBAL VARIABLES ARE BAD. It's not something I invented. It's well > > accepted that proper scoping yields better code. > > FOLLOWING UPERCASE MAXIMS BLINDLY IS BAD. (and that's something I > have invented.) > > > > > The way we use those two files shows how much of a mess globally > > accessible variable can be. > > > > Not putting paths to executable in constants.py > > ----------------------------------------------- > > Instead we should have a module for each utility executable. The > > module > > should wrap the utility with a pythonic interface. That way there > > is no > > reason for more then one module to have access to a utility's path. > > > > For example: > > > > cat.py > > ------ > > > > _CAT_PATH = @CAT_PATH@ > > > > def cat(files): > > cmd = [_CAT_PATH] + files > > out, err = exec(cmd) > > return out > > > > Now there is no need to know where cat is. You have a proper > > interface > > to use. Preventing people from re-wrapping the utility over and > > over. > > This in turn means you have a canonical wrapper that is more likely > > to work. > > I agree that our current behavior is clunky. Most external executable > should be > wrapped by functions that exposes the functionality which we need > from them. > > I am not sure that having a module per executable makes the most > sense; I'd > separate modularity based on Vdsm usage of them. Anyway, the only up > side of our > current constants-aggregating approach, is that the number of files > editted by > autoconf is limitted. > > P.S. EXT_CAT is a horrible example. It exists only for letting Vdsm > read > /etc/multipath.conf. This functionality must be wrapped properly and > shoved into > supervdsm.
That doesn't mean that supervdsm shouldn't have a canonical way of using it which could be later on leveraged in other places. i.e. assuming we converge on this way of doing things, it should apply to supervdsm as well. > > > > > Every other constant > > -------------------- > > Other constants should be moved to their proper modules and used > > only > > when importing the module makes sense this will give you another > > step > > where you make sure your decencies are correct. > > > > Configuration > > ------------- > > > > Configuration files are just parameters to the program. They should > > be > > treated as such. Their scope should not leave the main class of the > > project. In our case VDSM is actually 2 projects meshed together > > like > > Siamese twins so I think clientIf.py and hsm.py should be the only > > ones > > with access to the config. Everything else should expect to get > > those > > values in a regular programmatic fashion. > > > > > > In my opinion these changes will make VDSM a lot more flexible, > > will > > make more parts independent and easily separated and at that end > > help us > > make VDSM better for generations to come. > > In my opinion our use of config.config is perfectly fine. The > configuration file > is a single global object, and it makes sense to keep its Python > representation > as a single global object. I'd like keeping it this way because if Vm > migration > process needs a new parameter (say, when should we decide that > migration is not > going anywhere) MigrationSourceThread can simply read it from the > config. If we > take your approach, we would have to trickle the value all the way > from clientIF > through Vm down to the specific function that needs it. It would > clutter the > code and would not scale nicely with the number of configurational > parameters. Actually, already today we should be accepting most if not all "global" params from the management system so that in multi-node environments we would be able to centrally manage the configurations (e.g. nfs mount params). Most of the params should be kept the same across the cluster so these should trickle down from management through ctors to wherever. Unlike Saggy, I'm not convinced yet that globals are *always* bad, but they do tend to make the code less clear and allow one not to think about what the proper interfaces for methods should be. > > > > > > You can help! > > ------------- > > > > * Start by taking you favorite utility and write a proper safe > > module > > around it. > > * When writing new code try and not access the configuration > > directly. > > * Modules are free (really) don't just put stuff in misc\utils try > > and > > put some order to things. > > > > _______________________________________________ > > vdsm-devel mailing list > > vdsm-devel@lists.fedorahosted.org > > https://fedorahosted.org/mailman/listinfo/vdsm-devel > _______________________________________________ > vdsm-devel mailing list > vdsm-devel@lists.fedorahosted.org > https://fedorahosted.org/mailman/listinfo/vdsm-devel > _______________________________________________ vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/vdsm-devel
