On Thu, 2010-02-11 at 23:18 -0800, K.D. Lucas wrote: > Lucas, > > Thanks for your comments. I'll get a good svn diff patch out soon. > > Before I do though, just one question. I kind of patterned my > get_value off of the same thing that is done in <AT_ROOT>/tko/db.py, > so that is where I got the idea for that. I wanted to kind of shorten > the path to this function as it would really make the code look ugly > since the constant is quite long to begin with.
Hmm, you are right. I guess in this case that is OK, just keep it the way it was on your patch. > Also, is svn diff ok to use or would you prefer git? svn diff is totally OK, I just suggested git because I find it easier. Just go ahead and do it on svn. > Kelly > > On Thu, Feb 11, 2010 at 6:31 PM, Lucas Meneghel Rodrigues > <[email protected]> wrote: > On Thu, 2010-02-11 at 17:44 -0800, K.D. Lucas wrote: > > I'm currently working with a class of machines with very > limited disk > > space; consequently, I'd like to move the following constant > to a > > place where it is a little more convenient to configure. > > > Hi, some comments below: > > > I'd like to move AUTOTEST_GB_DISKSPACE_REQUIRED = 20 to a > value > > derived from global_config.ini file. > > > > > > --- global_config.ini (revision 4230) > > +++ global_config.ini (working copy) > > > Your mailer chewed up your patch, you'll have to find a way to > make it > not auto-wrap the patch contents. > > > [SERVER] > > +# Minimum amount of disk space required for AutoTest in GB > > +gb_diskspace_required: 5 > > > ===================================================================== > > and then implement the following change in > > <AUTOTEST_ROOT>/server/hosts/abstract_ssh.py > > > Point out manually where the change is is also not a good > practice, you > should generate the contents of the patch all at once, p1 > level, using > an appropriate tool such as svn diff, or better, git. > > If you are interested I can send you a quick start guide of > how to > re-create this patch using git and send it with git > send-email. It will > resolve all the formatting problems on your current patch. > > > > ====================================================================== > > --- abstract_ssh.py (revision 4230) > > +++ abstract_ssh.py (working copy) > > @@ -5,11 +5,12 @@ > > from autotest_lib.client.common_lib.global_config import > > global_config > > > > > > -enable_master_ssh = > global_config.get_config_value('AUTOSERV', > > - > > 'enable_master_ssh', > > - > type=bool, > > default=False) > > +get_value = global_config.get_config_value > > > It'd be preferable to not do the above, as we keep this import > style to > make it easier for unit tests, and also, get_value is a very > generic > name, which is a bad idea in terms of namespace. > > > +enable_master_ssh = get_value('AUTOSERV', > 'enable_master_ssh', > > type=bool, > > + default=False) > > > > + > > def make_ssh_command(user="root", port=22, opts='', > > hosts_file='/dev/null', > > connect_timeout=30, > alive_interval=300): > > base_command = ("/usr/bin/ssh -a -x %s -o > > StrictHostKeyChecking=no " > > @@ -487,9 +488,11 @@ > > > > > > # tunable constants for the verify & repair code > > - AUTOTEST_GB_DISKSPACE_REQUIRED = 20 > > + AUTOTEST_GB_DISKSPACE_REQUIRED = get_value("SERVER", > > + > > "gb_diskspace_required", > > + type=int, > > + default=20) > > > > - > > def verify_connectivity(self): > > super(AbstractSSHHost, self).verify_connectivity() > > > > > The general approach looks good to me, but you have to fix the > issues > found and re-send so we can consider your patch for inclusion. > > Lucas > > > > > > -- > K.D. Lucas > [email protected] _______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
