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
_______________________________________________
Autotest mailing list
[email protected]
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
