Skip to content
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

fix minor issues with debug and item labels #41331

Merged
merged 5 commits into from
Jun 21, 2018
Merged

Conversation

bcoca
Copy link
Member

@bcoca bcoca commented Jun 8, 2018

SUMMARY
  • no more item=None, we always have a label now
  • debug should only show expected information, either msg= or the var in var=
  • also fixed method name, deprecated misleading _get_item

fixes #41571

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

callbacks

ANSIBLE VERSION
>=2.5

@ansibot ansibot added affects_2.7 This issue/PR affects Ansible v2.7 bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests. labels Jun 8, 2018
@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Jun 8, 2018
@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jun 14, 2018
@bcoca bcoca added this to To Do in 2.5.x blocker list via automation Jun 15, 2018
@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jun 15, 2018
@bcoca bcoca requested review from agaffney and sivel June 15, 2018 20:14
return item

def _get_item(self, result):
''' here for backwards compat, really should have always been named: '''
self._display.deprecated("This callback should be updated to use the _get_item_label method instead", version="2.11")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do better than "This callback"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only for v2 ones, older ones are not guaranteed to have name

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could fall back to __file__ or something in that case at minimum. Could be useful to include that regardless.

for key in list(result.keys()):
if key.startswith('_'):
continue # but leave 'control keys' in
if key != 'msg':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These 2 if statements could probably be combined. To pop when if it doesn't start with _ or not equal to msg.

 - no more `item=None`, we always have a label now
 - debug should only show expected information, either msg= or the var in var=
 - also fixed method name, deprecated misleading _get_item
@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jun 15, 2018
return item

def _get_item(self, result):
''' here for backwards compat, really should have always been named: '''
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this comment is missing a _get_item_label at the end, unless that colon implies that it should be inferred from reading the code below it.

label = self._task.loop_control.label

# ensure we always have a label
if not label:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be if label is None:, as the above would be triggered by doing label: false (even though it's somewhat non-sensical).

@agaffney
Copy link
Contributor

I'm not entirely sure what this is actually fixing, but it looks good in general

@ansible ansible deleted a comment from ansibot Jun 20, 2018
@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jun 21, 2018
@bcoca bcoca merged commit 27c43da into ansible:devel Jun 21, 2018
@bcoca bcoca deleted the debug_fixes branch June 21, 2018 20:15
bcoca added a commit to bcoca/ansible that referenced this pull request Jun 21, 2018
* fix minor issues with debug and item labels

 - no more `item=None`, we always have a label now
 - debug should only show expected information, either msg= or the var in var=
 - also fixed method name, deprecated misleading _get_item

(cherry picked from commit 27c43da)
bcoca added a commit to bcoca/ansible that referenced this pull request Jun 21, 2018
* fix minor issues with debug and item labels

 - no more `item=None`, we always have a label now
 - debug should only show expected information, either msg= or the var in var=
 - also fixed method name, deprecated misleading _get_item

(cherry picked from commit 27c43da)
mattclay pushed a commit that referenced this pull request Jun 21, 2018
* fix minor issues with debug and item labels

 - no more `item=None`, we always have a label now
 - debug should only show expected information, either msg= or the var in var=
 - also fixed method name, deprecated misleading _get_item

(cherry picked from commit 27c43da)
jacum pushed a commit to jacum/ansible that referenced this pull request Jun 26, 2018
* fix minor issues with debug and item labels

 - no more `item=None`, we always have a label now
 - debug should only show expected information, either msg= or the var in var=
 - also fixed method name, deprecated misleading _get_item
kbreit pushed a commit to kbreit/ansible that referenced this pull request Jul 3, 2018
* fix minor issues with debug and item labels

 - no more `item=None`, we always have a label now
 - debug should only show expected information, either msg= or the var in var=
 - also fixed method name, deprecated misleading _get_item

(cherry picked from commit 27c43da)
nitzmahone pushed a commit that referenced this pull request Jul 3, 2018
* fix minor issues with debug and item labels

 - no more `item=None`, we always have a label now
 - debug should only show expected information, either msg= or the var in var=
 - also fixed method name, deprecated misleading _get_item

(cherry picked from commit 27c43da)
@ansible ansible locked and limited conversation to collaborators Jun 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.7 This issue/PR affects Ansible v2.7 bug This issue/PR relates to a bug. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Noisy output using 'debug: var=item.' with 'with_items'
4 participants