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

deprecated-local-action: refactor to use matchtask() #2238

Merged
merged 7 commits into from Jul 19, 2022

Conversation

nishipy
Copy link
Contributor

@nishipy nishipy commented Jul 14, 2022

A part of #2105.

This PR fixes the following points for deprecated-local-action:

  • uses match() to look at each line
  • can get false positives in comments or strings
  • does not benefit from any of the additional info that gets added by other standard methods

Signed-off-by: nishipy <goodisonev4@gmail.com>
Signed-off-by: nishipy <goodisonev4@gmail.com>
Signed-off-by: nishipy <goodisonev4@gmail.com>
@nishipy nishipy requested review from a team as code owners July 14, 2022 02:47
@nishipy nishipy changed the title Refactor deprecated local action deprecated-local-action: refactor to use matchtask() Jul 14, 2022
Signed-off-by: nishipy <goodisonev4@gmail.com>
@nishipy nishipy force-pushed the refactor-deprecated-local-action branch from 29d7438 to 0f97a0e Compare July 14, 2022 03:02
Signed-off-by: nishipy <goodisonev4@gmail.com>
Copy link
Member

@cognifloyd cognifloyd left a comment

Choose a reason for hiding this comment

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

Nice! I left a comment about a possible alternative implementation. This is exciting to see all these improvements! Thank you!


# check if module invocated with old style
old_style_keys = ("local_action",)
result = _extract_old_style_keys_from_task(result, sanitized_task, old_style_keys)
Copy link
Member

Choose a reason for hiding this comment

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

This is kind of cool.

But, we might not need to change any of the utils because of another newish feature: needs_raw_task.

If the rule class has needs_raw_task = True as a class level variable, then matchtask gets passed the raw/unparsed task in task["__raw_task__"].

Would that work here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! Using task["__raw_task__"] looks smart and enough for us. Let me fix that.

SUCCESS_TASK = """
- name: task example
boto3_facts:
delegate_to: localhost # local_action
Copy link
Member

Choose a reason for hiding this comment

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

Sweet example!

Signed-off-by: nishipy <goodisonev4@gmail.com>
@ssbarnea ssbarnea added the bug label Jul 19, 2022
@ssbarnea ssbarnea merged commit 659ab7c into ansible:main Jul 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants