Ok, patch is attached.
On Fri, Feb 12, 2010 at 9:53 AM, Lucas Meneghel Rodrigues
<[email protected]>wrote:
> On Fri, Feb 12, 2010 at 3:51 PM, K.D. Lucas <[email protected]> wrote:
> > Ok, let me know if you want me to redo the patch with the short function
> > name.
>
> That's OK, you can use the short function name.
>
> > On Fri, Feb 12, 2010 at 2:57 AM, Lucas Meneghel Rodrigues <
> [email protected]>
> > wrote:
> >>
> >> 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]
> >>
> >>
> >
> >
> >
> > --
> > K.D. Lucas
> > [email protected]
> >
> > _______________________________________________
> > Autotest mailing list
> > [email protected]
> > http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
> >
> >
>
>
>
> --
> Lucas
>
--
K.D. Lucas
[email protected]
Index: global_config.ini
===================================================================
--- global_config.ini (revision 4240)
+++ global_config.ini (working copy)
@@ -42,6 +42,8 @@
# Number of old logs to keep around
rpc_num_old_logs: 5
rpc_max_log_size_mb: 20
+# Minimum amount of disk space required for AutoTest in GB
+gb_diskspace_required: 5
# If for some reason you don't want to rely on the Mail Transport Agent
# installed on this machine, you can provide an SMTP server directly here.
# If none provided, defaults to 'localhost', which tries to use the MTA
Index: server/hosts/abstract_ssh.py
===================================================================
--- server/hosts/abstract_ssh.py (revision 4240)
+++ server/hosts/abstract_ssh.py (working copy)
@@ -5,9 +5,9 @@
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
+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',
@@ -487,7 +487,10 @@
# 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):
_______________________________________________
Autotest mailing list
[email protected]
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
