Fix bug in site_host_attributes.

Fix bug in site_host_attributes.

the code path which is executed on autotest server (inside scheduler) is slightly different than run_remote_hosts, so it is not caught until deployed.

[kmixter1, 2010-05-03 16:56:20]

http://codereview.chromium.org/1837002/diff/2001/3001
File server/site_host_attributes.py (right):

http://codereview.chromium.org/1837002/diff/2001/3001#newcode83
server/site_host_attributes.py:83: host_obj.labels.all()])
Oops - sorry about that.

http://codereview.chromium.org/1837002/diff/2001/3001#newcode92
server/site_host_attributes.py:92: if value:
I don't understand what this is doing.  At very least it's saying it's ignoring
attribute strings when it's not and it seems to take:

X,<anything-1>,<anything-2>

and map X to <anything-2> which seems odd (because type(splitnames[1]) always is
'str').  The intention was to allow X to map to strings when you said string,
and enable extending it to map to integers, etc in the future.  I see a bug in
the original code.

[kmixter1, 2010-05-03 16:57:45]

(I meant to say "I don't see a bug in the original code.")

On Mon, May 3, 2010 at 9:56 AM,  <kmixter@chromium.org> wrote:
>
> http://codereview.chromium.org/1837002/diff/2001/3001
> File server/site_host_attributes.py (right):
>
> http://codereview.chromium.org/1837002/diff/2001/3001#newcode83
> server/site_host_attributes.py:83: host_obj.labels.all()])
> Oops - sorry about that.
>
> http://codereview.chromium.org/1837002/diff/2001/3001#newcode92
> server/site_host_attributes.py:92: if value:
> I don't understand what this is doing.  At very least it's saying it's
> ignoring attribute strings when it's not and it seems to take:
>
> X,<anything-1>,<anything-2>
>
> and map X to <anything-2> which seems odd (because type(splitnames[1])
> always is 'str').  The intention was to allow X to map to strings when
> you said string, and enable extending it to map to integers, etc in the
> future.  I see a bug in the original code.
>
> http://codereview.chromium.org/1837002/show
>

[ericli, 2010-05-03 17:03:16]

http://codereview.chromium.org/1837002/diff/2001/3001
File server/site_host_attributes.py (right):

http://codereview.chromium.org/1837002/diff/2001/3001#newcode92
server/site_host_attributes.py:92: if value:
We have labels which has no comma, e.g. netbook_TYPE, board_TYPE.

And in that case, there is no splitnames[1] at all. And I guess I guess I could
safely assume I only need to convert value when it exists.


On 2010/05/03 16:56:21, kmixter1 wrote:
> I don't understand what this is doing.  At very least it's saying it's
ignoring
> attribute strings when it's not and it seems to take:
> 
> X,<anything-1>,<anything-2>
> 
> and map X to <anything-2> which seems odd (because type(splitnames[1]) always
is
> 'str').  The intention was to allow X to map to strings when you said string,
> and enable extending it to map to integers, etc in the future.  I see a bug in
> the original code.
>

[kmixter1, 2010-05-03 17:13:17]

LGTM

http://codereview.chromium.org/1837002/diff/2001/3001
File server/site_host_attributes.py (right):

http://codereview.chromium.org/1837002/diff/2001/3001#newcode92
server/site_host_attributes.py:92: if value:
On 2010/05/03 17:03:16, ericli wrote:
> We have labels which has no comma, e.g. netbook_TYPE, board_TYPE.
> 
> And in that case, there is no splitnames[1] at all. And I guess I guess I
could
> safely assume I only need to convert value when it exists.
> 
> 
> On 2010/05/03 16:56:21, kmixter1 wrote:
> > I don't understand what this is doing.  At very least it's saying it's
> ignoring
> > attribute strings when it's not and it seems to take:
> > 
> > X,<anything-1>,<anything-2>
> > 
> > and map X to <anything-2> which seems odd (because type(splitnames[1])
always
> is
> > 'str').  The intention was to allow X to map to strings when you said
string,
> > and enable extending it to map to integers, etc in the future.  I see a bug
in
> > the original code.

I see, the bug was when there is no comma the final case raises IndexError.  Can
you change it to:

value=','.join(splitnames[1:])
if len(splitnames) == 1:
  continue
elif value == 'True'
...
elif value == 'False'
...
elif splitnames[1] == 'string':
  log.info('Non-attribute...')
  continue

[kmixter1, 2010-05-03 17:56:20]

LGTM