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

Add hostvars mapping in awx_display callback #1181

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

Akasurde
Copy link
Member

SUMMARY

Fixes: #1137

Signed-off-by: Abhijeet Kasurde [email protected]

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ansible_runner/display_callback/callback/awx_display.py

@Akasurde Akasurde requested a review from a team as a code owner January 17, 2023 10:07
@Akasurde
Copy link
Member Author

cc @sivel

@AlanCoding
Copy link
Member

We should add some regression test coverage, but 👍 this looks good.

Copy link
Member

@nitzmahone nitzmahone left a comment

Choose a reason for hiding this comment

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

This needs to delegate to core's encoder- it's already missing a bunch of object types that core outputs can contain today (and are handled by core's encoder), and once data tagging ships, it'll be hopelessly broken.

@AlanCoding
Copy link
Member

The encoder class is pretty self-aware that it duplicates the Ansible-core version, in its doc string:

The class AnsibleJSONEncoder exists in Ansible core for this function this performs a mostly identical function via duck typing

We should really fix this soon, so I want to formalize the suggestion. Is the idea to replace the AnsibleJSONEncoderLocal class inside of ansible_runner.display_callback.callback.awx_display with instead an import to this?

from ansible.module_utils.common.json import AnsibleJSONEncoder

@nitzmahone
Copy link
Member

Yeah, probably something along those lines, at least for this specific case, but especially once data tagging lands, we'll need to be much more specific about what metadata we actually want to preserve in the callback output.

I'm still uncertain about the value of e.g., preserving the internal representation of an inline vaulted var in callback output. Shouldn't we just mask the value? I don't know what people typically do with the serialized output details like this beyond just looking at it in the UI, and we're only providing special handling for an uncommon subset of vault stuff today... Data tagging will provide us with a whole lot more choices to make about this, but also a lot more flexibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Object of type HostVarsVars is not JSON serializable
3 participants