Yeah, I agree I should rename get_all_label to get_all_labels. Consider that
is done.

Thanks.

On Thu, Sep 16, 2010 at 7:43 AM, John Admanski <[email protected]> wrote:

> 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
>>
>>
>>
>


-- 
Eric Li
李咏竹
Google Kirkland
_______________________________________________
Autotest mailing list
[email protected]
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest

Reply via email to