It looks good to me. Almost any kind of quoting would work so long as the
commas in labels get quoted, using URL encoding seems reasonable. I might
call it get_all_labels instead of get_all_label.

I don't know if jamesren want to comment since it touches the scheduler as
well.

-- John

On Wed, Sep 15, 2010 at 10:30 AM, Eric Li(李咏竹) <[email protected]> wrote:

> From 6fd00c819a5927dc54645fe7acd5f16cae5b4e34 Mon Sep 17 00:00:00 2001
> From: Eric Li <[email protected]>
> Date: Wed, 15 Sep 2010 10:19:33 -0700
>  Subject: [PATCH] Read all labels from host_keyvals file and handle the
> comma (,) within labels properly.
>
> Chromium OS autotest project had assigned labels with comma to test hosts
> and this situation makes the current host_keyvals file impossible to parse
> out correct label values. This change will fix/enhance that.
>
> I am not aware of any other usage of host_keyvals in the source tree, and
> if you let me know I would be very happy to fix those too.
>
> Thanks.
>
> Signed-off-by: Eric Li <[email protected]>
> ---
>  scheduler/monitor_db.py |    3 ++-
>  server/hosts/remote.py  |   17 ++++++++++++++++-
>  2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/scheduler/monitor_db.py b/scheduler/monitor_db.py
> index 5b7e593..2d878bb 100755
> --- a/scheduler/monitor_db.py
> +++ b/scheduler/monitor_db.py
> @@ -7,7 +7,7 @@ Autotest scheduler
>
>  import common
>  import datetime, errno, optparse, os, pwd, Queue, re, shutil, signal
> -import smtplib, socket, stat, subprocess, sys, tempfile, time, traceback
> +import smtplib, socket, stat, subprocess, sys, tempfile, time, traceback,
> urllib
>  import itertools, logging, weakref, gc
>
>  import MySQLdb
> @@ -1823,6 +1823,7 @@ class TaskWithJobKeyvals(object):
>          keyval_path = os.path.join(self._working_directory(),
> 'host_keyvals',
>                                     host.hostname)
>          platform, all_labels = host.platform_and_labels()
> +        all_labels = [ urllib.quote(label) for label in all_labels ]
>          keyval_dict = dict(platform=platform, labels=','.join(all_labels))
>          self._write_keyvals_before_job_helper(keyval_dict, keyval_path)
>
> diff --git a/server/hosts/remote.py b/server/hosts/remote.py
> index 55be848..227c170 100644
> --- a/server/hosts/remote.py
> +++ b/server/hosts/remote.py
> @@ -1,7 +1,7 @@
>  """This class defines the Remote host class, mixing in the SiteHost class
>  if it is available."""
>
> -import os, logging
> +import os, logging, urllib
>  from autotest_lib.client.common_lib import error
>  from autotest_lib.server import utils
>  from autotest_lib.server.hosts import base_classes, bootloader
> @@ -220,6 +220,21 @@ class RemoteHost(base_classes.Host):
>              return None
>
>
> +    def get_all_label(self):
> +        """
> +        Return all labels, or empty list if label is not set.
> +        """
> +        if self.job:
> +            keyval_path = os.path.join(self.job.resultdir, 'host_keyvals',
> +                                       self.hostname)
> +            keyvals = utils.read_keyval(keyval_path)
> +            all_labels = keyvals.get('labels', '')
> +            if all_labels:
> +              all_labels = all_labels.split(',')
> +              return [urllib.unquote(label) for label in all_labels]
> +        return []
> +
> +
>      def delete_tmp_dir(self, tmpdir):
>          """
>           Delete the given temporary directory on the remote machine.
> --
> 1.7.1
>
>
>
_______________________________________________
Autotest mailing list
[email protected]
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest

Reply via email to