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

win_environment : Maybe add an option to read variable content ? #605

Open
freeeflyer opened this issue Apr 18, 2024 · 3 comments
Open

win_environment : Maybe add an option to read variable content ? #605

freeeflyer opened this issue Apr 18, 2024 · 3 comments

Comments

@freeeflyer
Copy link

SUMMARY

win_environment only modify var. We could add an option to return the current value.

ISSUE TYPE
  • Feature Idea
COMPONENT NAME

We could only return the value when no actual value is given

ADDITIONAL INFORMATION
- name: Set an environment variable for all users
  ansible.windows.win_environment:
    state: present
    name: TestVariable

Return value of TestVariable

@jborean93
Copy link
Collaborator

I don't see a real benefit of doing this over a simple call to setup to get the builtin facts or just doing a win_shell command to get the env var you need.

- hosts: windows
  gather_facts: false
  tasks:
  - name: Gather all env vars
    setup:
      gather_subset: env

  - name: Display env var through facts
    debug:
      var: ansible_env['SystemRoot']

  - name: get env var through win_shell
    win_shell: $env:SystemRoot
    register: shell_res
    changed_when: False

  - name: Display env var through win_shell
    debug:
      var: shell_res.stdout | trim  # Or shell_res.stdout_lines[0]

The ansible_env/ansible_facts['env'] option will actually be automatically retrieved if you haven't disabled fact gathering by default. It's a bit heavy handed but it does retrieve all the env vars. The win_shell example is a bit simpler as you are just retrieving an env var on demand, the only two caveats are that you need to do changed_when: False on the task if you care about reported changes and access the value through shell_res.stdout | trim or shell_res.stdout_lines[0].

We try to avoid having modules that both set a state and get a state in favour of separate modules. While we do have some today that do that it's more of a historical thing that happened before we put those rules in place.

@freeeflyer
Copy link
Author

We try to avoid having modules that both set a state and get a state in favour of separate modules. While we do have some today that do that it's more of a historical thing that happened before we put those rules in place.

Why is that ?

@jborean93
Copy link
Collaborator

It makes the module development a lot easier to handle as they stay dedicated to one task and one task only. For example

  • You no longer have to deal with returning values that could differ if you are in check mode or not after making a change
  • The module's documentation is no longer as complex, it's easier to document options that are required all the time rather than if x is set to y and so on
  • It more easily allows us to expose filtering options as part of the module options, trying to do that in a module that also makes changes complicated the UX and makes things more confusing for people

While yes this is a pretty simple thing to add support for I don't believe the benefits outweigh the extra complexity in the code, especially since the setup module already retrieves all env vars and you can get your own env var with a simple win_shell task.

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

No branches or pull requests

2 participants