-
Notifications
You must be signed in to change notification settings - Fork 227
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
remove automatic resolver #112
Conversation
Pull Request Test Coverage Report for Build 531
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. I just had one question about one of the changes.
assert result | ||
for h, r in result.items(): | ||
assert h == r.stdout.strip() | ||
assert h == r[1].stdout.strip() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seemed a bit strange...why the addition of the [1] i.e. why did it go from r.stdout.strip()
to r[1].stdout.strip()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I understand this behavior now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for reference; the reason is because instead of calling a task directly we are calling now a grouped task. Grouped tasks return a dict of results (key is host) with a list of results of which the first element is the result for the whole group.
It was used very inconsistently so let's remove it. If we want it later on we can try to add it via a decorator or something like that.
Solves #107