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

Adding parameter to prefix option with spaces for ini_file #8639

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

skinnayt
Copy link

SUMMARY

Add parameter to prefix option with spaces when formatting the options in a section.

This is useful for formatting some INI files. The use case here was specifically around the updating some options/values in an InfluxDB config file. This is usually in an users directory. (/home/user/.influxdbv2/configs)

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ini_file module - new parameter option_prefix_spaces

ADDITIONAL INFORMATION

With an existing InfluxDB configs file of the following:

[default]
  url = "http://localhost:8086"
  token = "someSuperSecretTokenThatIsLong"
  org = "myorg"

Using the below task:

- name: Update url in InfluxDB user config
  community.general.ini_file:
    path: /home/user/.influxdbv2/configs
    section: default
    option: url
    value: https://localhost:8086

produces the following in the configs file:

[default]
url = "https://localhost:8086"
token = "someSuperSecretTokenThatIsLong"
org = "myorg"

While this seems okay, the next time that the influx or influxd commands are used, it reformats the file the to the format that it expects. Running ansible again tries to change the file as it notices that the current content is different than what the expected content will be after running the task

Adding the new ini_file parameter allows the change to be applied once and not affect now the influx/influxd commands formats the file.

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added WIP Work in progress feature This issue/PR relates to a feature request integration tests/integration module module new_contributor Help guide this first time contributor plugins plugin (any type) tests tests labels Jul 15, 2024
@skinnayt skinnayt marked this pull request as ready for review July 15, 2024 04:59
@ansibullbot ansibullbot removed the WIP Work in progress label Jul 15, 2024
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-9 Automatically create a backport for the stable-9 branch labels Jul 15, 2024
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

It seems you want this option to use the ini_file module to operate on TOML files. I don't think it's a good idea to use the ini_file module for TOML files, and I'm not sure whether adding this option to the ini_file module is a good idea since its main application seems to be to edit TOML files.

plugins/modules/ini_file.py Outdated Show resolved Hide resolved
plugins/modules/ini_file.py Outdated Show resolved Hide resolved
plugins/modules/ini_file.py Show resolved Hide resolved
@@ -257,6 +263,14 @@
value: xxxxxxxxxxxxxxxxxxxx
mode: '0600'
state: present
- name: Update the option and indent with spaces
community.general.ini_file:
path: /etc/influxdb/config.toml
Copy link
Collaborator

Choose a reason for hiding this comment

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

As the file extension indicates this is NOT an INI file, but a TOML file.

Using ini_file for a TOML file is generally not a good idea, and definitely should not appear in the examples to avoid suggesting this to users.

plugins/modules/ini_file.py Outdated Show resolved Hide resolved
@ansibullbot ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Jul 29, 2024
@ansibullbot ansibullbot removed the stale_ci CI is older than 7 days, rerun before merging label Jul 30, 2024
- name: Update the option and indent with spaces
community.general.ini_file:
path: /etc/influxdb/config.toml
path: /etc/influxdb/config.ini
Copy link
Collaborator

Choose a reason for hiding this comment

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

@felixfontein
Copy link
Collaborator

It seems you want this option to use the ini_file module to operate on TOML files. I don't think it's a good idea to use the ini_file module for TOML files, and I'm not sure whether adding this option to the ini_file module is a good idea since its main application seems to be to edit TOML files.

This is the main problem with this PR, could you please comment on this? Are you aware of any actual INI files (which are not TOML files!) that need this indentation?

@ansibullbot ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Aug 13, 2024
@skinnayt
Copy link
Author

It seems you want this option to use the ini_file module to operate on TOML files. I don't think it's a good idea to use the ini_file module for TOML files, and I'm not sure whether adding this option to the ini_file module is a good idea since its main application seems to be to edit TOML files.

This is the main problem with this PR, could you please comment on this? Are you aware of any actual INI files (which are not TOML files!) that need this indentation?

I didn't find any other "module" in Ansible that provided the framework for handling this file. I do understand that this might not be the correct path forward. I can move the example back to the file extension of "toml" for the documentation.

I have looked around for modules to handle TOML and the current existing python ones don't do writing to a file very well. I will probably have to dig into the code to get it working.

If you would rather close this out and wait for a proper fix in a new module for handling toml code, I can make some attempts at that.

@felixfontein
Copy link
Collaborator

When searching a bit I found https://github.com/python-poetry/tomlkit which seems to preserve style and comments. With that implementing an Ansible module for modifying TOML files should be possible.

@felixfontein felixfontein removed the backport-9 Automatically create a backport for the stable-9 branch label Oct 7, 2024
@felixfontein felixfontein added the backport-10 Automatically create a backport for the stable-10 branch label Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-10 Automatically create a backport for the stable-10 branch check-before-release PR will be looked at again shortly before release and merged if possible. feature This issue/PR relates to a feature request integration tests/integration module module new_contributor Help guide this first time contributor plugins plugin (any type) stale_ci CI is older than 7 days, rerun before merging tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants