Continue fix for 9871 #352
Conversation
|
Tests failed for this pull request. |
|
Code looks okay to me. The log.info message added produced some unexpected info on list |
|
argh wrong @. @zmc, thoughts? |
| machine) | ||
| # Delete this variable to avoid linter errors when we redefine it | ||
| # in a list comprehension below | ||
| del machine |
There was a problem hiding this comment.
instead of dancing around the linter, why not just use a different variable like 'fqdn' ?
There was a problem hiding this comment.
wasn't my code; I just factored it into the function. I wondered about that too. probably unnecessary altogether now that it's in a separate function, but I wasn't 100% sure what "linter" meant.
There was a problem hiding this comment.
I researched: I refer you to 883c97d :) but it can probably just disappear now that it's in a different function.
There was a problem hiding this comment.
repushed again and removed comment and del
|
Could you paste some sample output for posterity please? |
|
Also - I fixed the unit test (that I broke and is unrelated to this), so if you rebase you'll pick that up. |
|
$ teuthology-lock --brief |
dmick
force-pushed
the
wip-update-vm-hostkeys
branch
from
3fb604c to
344a80e
Compare
|
Tests passed for this pull request. |
|
rebased to master |
The existing logic was too tortured, and the previous fix was bad. Rewrite to extract "get_statuses()" to a worker, and make it extremely clear which VM hostkeys are being updated and why. Signed-off-by: Dan Mick <[email protected]>
dmick
force-pushed
the
wip-update-vm-hostkeys
branch
from
344a80e to
416528e
Compare
|
Tests passed for this pull request. |
The existing logic was too tortured, and the previous fix was bad.
Rewrite to extract "get_statuses()" to a worker, and make it
extremely clear which VM hostkeys are being updated and why.
Signed-off-by: Dan Mick [email protected]