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

feat(jinja): add macros.jinja file to accumulate common macros #134

Closed
wants to merge 1 commit into from
Closed

Conversation

noelmcloughlin
Copy link
Member

@noelmcloughlin noelmcloughlin commented Jun 1, 2019

This PR introduces a 1st revision of a common jinja/macros.j2 file to gather generic macros. The first revision includes a format_kwargs macro.

Background:
The format_kwargs macro is generic solution used in two formulas-

  • ceph-formula
  • postgres-formula

format_kwargs macro
This macro makes short work of rendering SLS lists which must be generated by mapping files and the switching of grains.

Use Case

osfamilymap.yaml

Debian:
  pkg:
    repo:
      name: 'deb https://packages.template.org/debian stable main'
      key_url: https://packages.template.org/debian/template.org.key
      file: file: /etc/apt/sources.list.d/template.list
      dist: stable

RedHat:
  pkg:
    repo:
      name: template-cli
      baseurl: https://packages.template.org/redhat
      key_url: https://packages.template.org/redhat/template.org.key
      enabledTrue: 1

package/install.sls

{%- from tplroot ~ "/jinja/macros.j2" import format_kwargs with context -%}

template-package-repo-managed:
  pkgrepo.managed:
    {{- format_kwargs(template.pkg.repo) }}
    - require_in:
      - pkg: template-package-install-pkg-installed

Copy link
Member

@myii myii left a comment

Choose a reason for hiding this comment

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

Just the doubled Jinja whitespace control, due to the linked comment.

template/jinja/macros.j2 Outdated Show resolved Hide resolved
@myii
Copy link
Member

myii commented Jun 2, 2019

Thanks for this, @noelmcloughlin. I think the idea itself is sound but we all need discuss how to proceed, with respect to consistency and a number of ongoing discussions:

  1. Using a jinja sub-directory; we haven't done so far, such as with libtofs.jinja but it's probably a good idea to separate these libraries somewhere.
  2. Using the .j2 file extension; we're primarily using .jinja so far; when's the right time to use .j2 and the right time to use .jinja? Is it worth using one for libraries and the other for templates?
  3. Should we group numerous macros under macros.xyz or should we have separate libraries for each one, with consideration to spreading updates across numerous formulas?
  4. How should we implement this with regards to [Discussion] [Idea] Move all utilities for formulas outside of edited files (states, yaml) #122?
  5. Any decisions here will also affect chore(libsaltcli): add lib to check type of Salt command being used #131.

If we're going to follow our current implementation scheme, then this should be something like:

{%- from tplroot ~ "/libkwarg.jinja" import format_kwargs with context %}

However, I am not proposing that we continue with this scheme.

@noelmcloughlin
Copy link
Member Author

Hi @myii

Using a jinja sub-directory; we haven't done so far, such as with libtofs.jinja but it's probably a good idea to separate these libraries somewhere.

macros are an advanced feature of a rendering language. It is probably a sound principle to hide the implementation since they are secondary libraries designed to cleanup overuse of jinja in SLS files.

when's the right time to use .j2 and the right time to use .jinja? Is it worth using one for libraries and the other for templates?

Fixed. Let's stick with jinja because the version is just an implementation detail.

Should we group numerous macros under macros.xyz or should we have separate libraries for each one, with consideration to spreading updates across numerous formulas?

There is no right answer. It's unclear if loading a single macro from a common library file incurs increased performance, compared to a modular library implementation. Buts It's a more simple implementation. We could start here and evolve.

This can wait until #122 and #131 become clearer.

@noelmcloughlin noelmcloughlin requested a review from daks June 2, 2019 10:59
@noelmcloughlin noelmcloughlin changed the title feat(jinja): add macros.j2 file to accumulate common macros feat(jinja): add macros.jinja file to accumulate common macros Jun 2, 2019
@noelmcloughlin
Copy link
Member Author

Here is another macro from prometheus-formula (only) which maybe portable too.

{#- Contactenate arguments #}
{%- macro concat_args(args) %}
{%-   set args = args|dictsort %}
{%-   if args|length > 0 %}
{%-     for k,v in args -%}
{%-       if not k or not v %}{% continue %}{% endif -%}
          --{{ k }}={{ v }}
{%-       if not loop.last %} {% endif -%}
{%-     endfor -%}
{%-   endif -%}
{%- endmacro %}

Anyway wait until #122 and #131 become clearer.

@noelmcloughlin
Copy link
Member Author

I'll close this as duplicate or #122

@pull-assistant
Copy link

Score: 1.00

Best reviewed: commit by commit


Optimal code review plan

     feat(jinja): add macros.j2 file to accumulate common macros

Powered by Pull Assistant. Last update c111bf2 ... c111bf2. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants