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