saltstack-formulas / template-formula

SaltStack formula template filled with dummy content
http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
119 stars 85 forks source link

map.jinja and yaml speed improvements on large formula's #24

Open aboe76 opened 5 years ago

aboe76 commented 5 years ago

see https://github.com/saltstack/salt/issues/39017

if we can convert some formula's which have a big or multiple yaml files we can convert these to json and import those.

oneliner to convert

python -c 'import sys, yaml, json; json.dump(yaml.load(sys.stdin), sys.stdout, indent=4)' < file.yaml > file.json

We could keep the yaml files for easier maintenance but have a nice readme about why this formula needs json files for importing...

myii commented 5 years ago

@aboe76 Sounds good! Thanks for adding that to the plan.


BTW, there's been a discussion on Slack about using the PillarStack_SDB Module alongside TOFS for even greater efficiency. I remember you mentioning that you use (or have used) PillarStack. I'm going to add this to the plan as something to be evaluated. Dominik Bessler has offered to give us some feedback about this combination as well. Any thoughts from your experience?

daks commented 5 years ago

So the idea is to use JSON instead of YAML because parsing is faster?

I'm not completely ok with keeping both JSON and YAML files with the same information inside, it will just burden the repository and introduce confusion. If we just want to ease maintenance, maybe add doc about converting JSON to YAML, edit YAML, and then convert YAML to JSON.

myii commented 5 years ago

@daks Thanks for sharing your views. Always helps when others share their expertise.

Just to be clear, @aboe76 is discussing this in relation to formulas which contain an excessive amount of YAML. It may be worth exploring the performance gains from using JSON in those cases. Just sharing ideas for the time being as we explore how SaltStack Formulas can be improved.

I agree with you about avoiding duplication at all costs. I'm sure we can find a solution for this if we decide that we would like to go forward with it.

noelmcloughlin commented 5 years ago

Many programmers consider YAML controversial and prefer JSON. This is another consideration.

myii commented 5 years ago

@noelmcloughlin added a decent comment on a PR that needs to be included here:

One more thought (per the vision stuff, not this PR) is that salt can support both yaml and json configuration. All formulas use yaml/ today but anticipating json/ or other data serialisation technology language in the design may make sense - something like this?

   template-formula/
        template/
          .. etc ..
             yaml/
                  defaults.yaml
                  osmap.yaml
                  osfamilymap.yaml
            json:
                 defaults.json
                 osmap.json
                 osfamilymap.json
alxwr commented 5 years ago

Maybe #116 is of interest here.

baby-gnu commented 5 years ago

Hello.

I personally completely prefer YAML since it's much more human readable and support comments.

My 2¢.

Regards.

myii commented 4 years ago

This conversation came up again in a Slack thread starting from this comment:

18h Nate Hello, everyone. I’ve been working with the SaltStack container to try and familiarize myself with Salt. One thing I’m having a hard time finding solid documentation on, is the structure of config files in /etc/salt/master.d/ The container writes your env var for master_config in JSON. However, I’ve seen some docs that state config files there need to be YAML. Does anyone have an opinion on how to handle those config files?

18h Imran Iqbal Simply copy the settings you'd like to override from /etc/salt/master and modify them in file(s) under /etc/salt/master.d.

17h Nate Thanks for the feedback! That makes sense. Do you know if the files can be both JSON and YAML?

17h Imran Iqbal I've never seen them in JSON format. Why would you want to override it like that? Wouldn't keeping it in the same format as /etc/salt/master be the simplest thing to do?

17h David Murphy you can feed them in JSON format since that is what the YAML parsed is turned into but it has been 4 years since done anything like that. Note YAML is a little easier to deal with, but JSON is doable

17h Nate I absolutely agree with both of you. To be honest, it’s been a slight struggle working with the container and converting to JSON. For reference, you can see the container taking JSON input and writing it to master.d. I may just move away from using the env vars, and use a dockerfile to add the config in as /etc/salt/master Thanks again for your input and insight! https://github.com/saltstack/saltdocker/blob/master/saltinit.py


32m Wayne Werner also FWIW putting them in JSON format eliminates the YAML parsing overhead. If you have a lot of things, using JSON can significantly improve startup times. I'm not sure what the exact figures are, but I know they're significant

28m Imran Iqbal @​Wayne Werner How much time would be saved with the default configuration files still provided in YAML? Or are you suggesting to effectively override entire configuration files for this performance gain?

20m Wayne Werner IIUC everything YAML in Salt gets parsed into a python dictionary, and JSON parsing is way faster than YAML. For basic config (<20-30 lines) this isn't a big deal. For a couple of thousand lines of pillar data & config, there's more advantage to using JSON ahead of time

5m Imran Iqbal Yes, I heard about this before. We were thinking about tackling this as some point: https://github.com/saltstack-formulas/template-formula/issues/24.

I've had a little thought about this. How about we still use YAML for development but we convert them to JSON for processing? We ensure that these generated files are also merged into the formula as artifacts to be consumed by map.jinja in production. A one-way conversion only, a bit like how JS files are minified, for performance gains. We could use the one-liner that @aboe76 provided in the first post of this issue.

dafyddj commented 4 years ago

Perhaps this could be done when packing into an spm.