Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

SEP 39: Allow use YAML dictionaries instead of arrays in Salt SLS files #62

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

MurzNN
Copy link

@MurzNN MurzNN commented Feb 17, 2022

Rendered proposal

This SEP is a continuation of tech debt issue saltstack/salt#61649

@MurzNN MurzNN requested a review from a team as a code owner February 17, 2022 10:26
@MurzNN MurzNN requested review from krionbsd and removed request for a team February 17, 2022 10:26
@welcome
Copy link

welcome bot commented Feb 17, 2022

Hello! Thank you for submitting a Salt Enhancement Proposal! Our process is detailed in the README.md and more about the SEP Life-cycle. An Open Core Team member will be assigned to follow up and help guide this SEP soon and you will find the this in the Community Slack channel #sep.
Please be sure to review our Code of Conduct.
You can also check out some of our community
resources:
- Community Wiki
- Salt’s Contributor Guide
- Join our Community Slack
- IRC on LiberaChat
- SaltStack YouTube channel
- SaltStackInc Twitch channel

@OrangeDog
Copy link

@whytewolf: this actually would take a bunch of deep level changes. mainly to the way module.function is evaluated between high and low states.
-- saltstack/salt#61649 (comment)

I don't think so. A renderer can just parse the yaml, then convert the dicts back to lists (e.g. via .items()).

The default yaml renderer could be changed to do it if it detects dicts, or a new yaml_dict renderer added.
People could do it right now with a custom renderer (e.g. in a salt-extension).

@MurzNN MurzNN changed the title Allow use YAML dictionaries instead of arrays in Salt SLS files SEP 62: Allow use YAML dictionaries instead of arrays in Salt SLS files Feb 17, 2022
@waynew
Copy link
Contributor

waynew commented Feb 18, 2022

@thatch45 may be able to weigh in on the historic reasons that arrays were chosen over dict.

It may have had something to do with order, but I'm not sure.

Honestly, I think the documentation would be the biggest hurdle here, and why if it were possible to support, I still would hesitate to accept this behavior. Even if the one style is weird, it's well-known. Unless you come to salt with YAML expertise and expect a 1:1 mapping of SLS files to Python, I don't think it's really that confusing.

I do agree that it's a bit confusing coming from other tools that use YAML in a particular way, though.

@whytewolf
Copy link

@whytewolf: this actually would take a bunch of deep level changes. mainly to the way module.function is evaluated between high and low states.
-- saltstack/salt#61649 (comment)

I don't think so. A renderer can just parse the yaml, then convert the dicts back to lists (e.g. via .items()).

The default yaml renderer could be changed to do it if it detects dicts, or a new yaml_dict renderer added. People could do it right now with a custom renderer (e.g. in a salt-extension).

the problem with the default yaml render getting involved at all,

the default yaml render is used a lot of places where that will change currently working behavior. such as pillar and default.yml. places were the returned information should be 1:1. in order to work with that the rendering would need to be context aware. which adds more processing of information into the renderer.

one of the arguments was that this would improve performance but in having to make it context aware would actually decrease performance.

to see an increase in performance you would have to do what i was saying and change the state behavior.

however introducing a new renderer that is not contextual could be used to let users make the choice either way. default would still be the same and work in the same way. to enable the behavior described set renderer: jinja|dict_yaml in the minion. would still need to set your #! renders in default files.

@OrangeDog
Copy link

(this isn't SEP 62 by the way, the SEP number is different to the PR number)

@dwoz dwoz changed the title SEP 62: Allow use YAML dictionaries instead of arrays in Salt SLS files SEP 39: Allow use YAML dictionaries instead of arrays in Salt SLS files Apr 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants