Ok, I've implemented all of changes you suggested, patch is attached.
Kelly
On Wed, Feb 17, 2010 at 12:40 PM, John Admanski <[email protected]>wrote:
> It would be good to also do an if self.job check, since people do at times
> use the host classes outside of running jobs and it would be good if the
> code can handle that case (the labels would just have to be None). In
> practice it's not a big deal since that code should never be calling
> host.get_platform_label() anyway but since the code is making an effort to
> deal with the fact that host keyvals may not be available it would be nice
> to handle that as well.
>
> Also, you could just do a return keyvals.get('platform', None) instead of
> having to use a big if-else block.
>
> -- John
>
>
> On Wed, Feb 17, 2010 at 10:17 AM, K.D. Lucas <[email protected]> wrote:
>
>> Good idea. I moved it into server/hosts/remote.py. Please take a look, as
>> this did simplify the function.
>>
>>
>>
>> On Tue, Feb 16, 2010 at 7:35 AM, John Admanski <[email protected]>wrote:
>>
>>> You should use the docstring format in CODING_STYLE.
>>>
>>> Also, I do like this better as a host method, not a general function. I
>>> suggest server/hosts/remote.py, there's no reason to tie it to the ssh
>>> classes specifically. Then you can even drop both of the arguments to the
>>> function, since they're implied by self.hostname and self.job.resultdir.
>>>
>>> -- John
>>>
>>> On Fri, Feb 12, 2010 at 1:05 PM, K.D. Lucas <[email protected]> wrote:
>>>
>>>> I wanted some jobs to select the server side component based on a
>>>> platform label, so this patch will add a function that returns the
>>>> platform_label of the specified hostname.
>>>>
>>>> Long term, I think it would be best to expose this as an attribute of
>>>> the AbstractSSHHost class, since it would then be very easy to access these
>>>> details and we wouldn't need to run any methods to grab them. I'm not
>>>> familiar enough yet with the code to understand how to best implement this,
>>>> so my work around is the attached patch.
>>>>
>>>> Kelly
>>>>
>>>> --
>>>> K.D. Lucas
>>>> [email protected]
>>>>
>>>> _______________________________________________
>>>> Autotest mailing list
>>>> [email protected]
>>>> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
>>>>
>>>>
>>>
>>
>>
>> --
>> K.D. Lucas
>> [email protected]
>>
>
>
--
K.D. Lucas
[email protected]
Index: server/hosts/remote.py
===================================================================
--- server/hosts/remote.py (revision 4241)
+++ server/hosts/remote.py (working copy)
@@ -199,6 +199,20 @@
return dir_name
+ def get_platform_label(self):
+ """
+ Return the platform label, or None if platform 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)
+ return keyvals.get('platform', None)
+ else:
+ return None
+
+
def delete_tmp_dir(self, tmpdir):
"""
Delete the given temporary directory on the remote machine.
_______________________________________________
Autotest mailing list
[email protected]
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
