Conversation
ansibot
added
WIP
s-hertel
force-pushed
the
ci_complete
branch
2 times, most recently
from
e4dd210 to
89f4d09
Compare
ansibot
added
docs
ansibot
removed
the
needs_ci
This comment has been minimized.
This comment has been minimized.
ansibot
added
test
Add changelog fragment for variables literally named "delegated_vars['ansible_ssh_host']" and "delegated_vars['ansible_host']"
|
|
||
| # TODO: consider deprecating or adding a better mechanism to pass | ||
| # variables to the results side. The comment above isn't really | ||
| # true since cvars is not templated. |
There was a problem hiding this comment.
I think it is fine to pass them w/o templating, so they can examine the sources 'as they were', what we should also do is pass the 'resolved' options with the connection itself
also we should look to deprecate callbacks that rely on variables directly, but we should ensure that the interface for getting the resolved information from the connection object is working.
in any case, the current fix seems to be fine, specially for backporting, but we want to get defined objects/interfaces for callbacks going forward.
There was a problem hiding this comment.
I'm not sure if there's a need. I overlooked that these are filtered out here https://github.com/ansible/ansible/blob/devel/lib/ansible/executor/task_result.py#L202, and after looking through the ansible-collections org, I don't see any callback plugins using anything other than "ansible_host" in the delegated variables.
How does #85397 look for backporting? I removed the PlayContext changes.
| - The host name potentially displayed by callback plugins or debug messages. | ||
| vars: | ||
| - name: inventory_hostname | ||
| - name: ansible_host |
There was a problem hiding this comment.
should we make this something shared? it seems something people would want to customize for most plugins
There was a problem hiding this comment.
Maybe a remote_addr option could be convenient? I thought the option name host_label would be better for local specifically though. This new option was solely to fix the local connection output with -vvv which can display a template string (since pc remote_addr may be one).
| def user(self) -> str: | ||
| if not (remote_user := self.get_option("remote_user")): | ||
| raise NotImplementedError | ||
| return remote_user |
There was a problem hiding this comment.
something to move to base class? probalby with play_context fallback?
There was a problem hiding this comment.
I don't think so, since the base methods do not use pc port or user, only the remote_addr. The ssh plugin sets these properties internally, and I was just trying to figure out if the ssh plugin doesn't have set_options reliably set (but I believe it does).
s-hertel
force-pushed
the
ci_complete
branch
8 times, most recently
from
a739718 to
5ea0e88
Compare
s-hertel
force-pushed
the
ci_complete
branch
from
02742ba to
397a189
Compare
7 participants
SUMMARY
ISSUE TYPE
COMPONENT NAME
ADDITIONAL INFORMATION