_syspaths.py variables defaulted unless explicitly changed

[alan-cugler]

Intent

This is a discussion issue of a possible PR to the "develop" branch of salt.

Description of Issue

Currently the _syspaths.py file has a set default of "None" for all variables listed.
You can set these variables to custom directories for individual parts of the salt infrastructure.

However, if you don't state all of the variables in this file the import in syspaths.py will result in an error like in #52228.

Proposed Change

The import line in the syspaths.py file should be modified to assume all non-mentioned variables default to "None" implicitly. This way the user can state only variables they are concerned with and ignore the rest whether they are there or not in the _syspaths.py file

A secondary addition would be to have syspaths.py add in the [missing variable] = None at the bottom of _syspaths.py for all variables missing from the import.

Related Issues

• salt break of _syspaths.py #52228
• docs page for _syspaths.py #51768

[nixjdm]

An additional motive for not having to declare attributes that would just be None in the custom _syspaths.py is that it would make an existing _syspaths.py more forwards compatible. If you have an existing setup where you need the custom module, and then upgrade Salt such that new attributes are looked for, it would likely just work instead of a getting an AttributeError.

I don't think we should do your second proposed change though. I don't think an existing _syspath.py that a user has customized or version controlled should be automatically written to. It's also not necessary especially if we get the first change.

[Ch3LL]

ping @saltstack/team-core can i get your opinions here?

[waynew]

On the face of it I agree with @nixjdm - seems like _syspath.py should be overriding the defaults.

I don't know if there were other more original ideas/reasons, though.

One counterpoint that I can think of - allowing missing values to pass silently could lead to problems with typos, or people missing out when new things are added that they may want to provide.

An approach that might be ideal is that if there are missing values that we provide a warning on startup suggesting that we'll set the value to None or whatever default value.

[alan-cugler]

@waynew, my secondary addition had also been thinking about users missing new variables, but your suggestion of a warning message would be a better way.

So if we are going to put a warning message then some thoughts for that:

• The warning message will be at the warning log level? as opposed to debug and info levels
• we can remove the traceback as long as we explicitly mention the _syspaths.py file and maybe its absolute path so someone knows where to find it.
• should we include a typo warning? or just list the variables being set to None and leave it to the user to discover their own typo from the listed variables?

Also keep in mind, we have agreed in another issue to create a _syspaths.py docs page to explain those variables, so that will be another avenue for users to see new variables being added as salt develops.

[waynew]

@alan-cugler Yeah, I think the warning log level would be ideal - because if we're operating under the assumption that if you're missing a value it's probably a mistake on the users part, then they should get informed about it. Especially if all it takes to silence the warning is just add Value = None to a file. Maybe we allow them to silence the warnings with a setting in _syspaths.py, or in the config. But honestly I think the best first approach would just require people to set the value.

I do like a typo warning - especially if there's no "normal" reason for them to have other values in that file. I'm particularly OK if we only check names and not _names, or we have a specific edit distance that we look at.

I like both the warning + the docs because I know not all users are reading docs or keeping up with release notes, and occasionally we miss some doc updates 😛 But perhaps the warning should link to the new docs page ;)

[alan-cugler]

Cool

• adjust syspaths.py import line
• warning level message:
  • possible typo?
  • lists variables missing
  • example line: [value] = None to make warning go away
  • URL link to relevant docs page (to be made)

[alan-cugler]

@waynew @Ch3LL can you put the assignee as me? I have started working on this and would hate for some else to also start working on the same thing.

[waynew]

@alan-cugler done ✨

[alan-cugler]

Made a pull request.
Did not create a URL link in the warning message.

[alan-cugler]

@waynew this PR is ready for merge and can close this issue once merged.