oxidized icon indicating copy to clipboard operation
oxidized copied to clipboard

no error, if the default config is created

Open twille007 opened this issue 1 year ago • 2 comments

Pre-Request Checklist

  • [ ] Passes rubocop code analysis (try rubocop --auto-correct)
  • [ ] Tests added or adapted (try rake test)
  • [ ] Changes are reflected in the documentation
  • [ ] User-visible changes appended to CHANGELOG.md

Description

If you use saltstack for example, on init oxidized, you will get an error, because it writes to stderr, if there is no configfile and if there is none, it creates one. It just tries to create the file, and if it doesn't work, it will write to stderr. Otherwise it will inform you about the new config file via stdout, and that you should edit it.

twille007 avatar Oct 10 '24 13:10 twille007

The whole Oxidized is based on using Oxidized.logger, defined in https://github.com/ytti/oxidized/blob/41f59ac25d4b7dec79216d79b90bd64899d68e39/lib/oxidized.rb#L39

You should use it and not puts.

Beside this, without knowing details about saltstack, I would think that someone deploying oxidized with saltstack knows that he/she also has to deploy a configuration with it.

robertcheramy avatar Oct 10 '24 18:10 robertcheramy

Ok, i changed it now to the logger.

And I completely agree that anyone deploying Oxidized with SaltStack should be aware that a configuration file needs to be deployed alongside it. However, I think it’s still a good practice not to raise an error if the configuration file is automatically created during the process. In cases where SaltStack (or any other deployment tool) handles the deployment, it’s useful to log a message indicating that the file was created, but raising an error could unnecessarily halt the process even when everything is working as expected.

twille007 avatar Oct 11 '24 13:10 twille007

I think this is a philosophy thing. When there is no config, oxidized will exit with exit code 1. And yes, this is an error.

robertcheramy avatar Nov 08 '24 20:11 robertcheramy