docker-formula icon indicating copy to clipboard operation
docker-formula copied to clipboard

Import YAML with context

Open Lucianovici opened this issue 3 years ago • 9 comments

Import with context to avoid errors like:

[CRITICAL] Rendering SLS ... failed: Jinja variable 'grains' is undefined
[CRITICAL] Rendering SLS ... failed: Jinja variable 'salt' is undefined

Lucianovici avatar May 17 '21 09:05 Lucianovici

@Lucianovici When amending the commit, please update the commit message for semantic-release purposes:

-Import YAML with context
+fix(map.jinja): import YAML with context

myii avatar May 19 '21 15:05 myii

@danny-smit I see you have some PRs active for this formula, which would benefit from this fix being applied. How about we get this PR merged soon, as one example of what would need to be done in case we're forced to patch the 30+ formulas?

myii avatar May 19 '21 15:05 myii

@myii Thanks, that sounds very good to me. Can I help with that?

danny-smit avatar May 19 '21 15:05 danny-smit

@myii Thanks, that sounds very good to me. Can I help with that?

@danny-smit Ah, I've just noticed that this PR was started from the contributor's master branch, so I don't think I can do the usual process of amending the PR myself. There are other ways using git but those don't work well on the GitHub side of things. Let's hope @Lucianovici is able to adjust the PR soon, so that we can get it merged.

myii avatar May 19 '21 16:05 myii

@danny-smit Actually, are you able to commit my suggestion from the review above? If so, I can finish the rest upon squash-merging this PR.

myii avatar May 19 '21 16:05 myii

@myii No, I don't have permission to add commits.

danny-smit avatar May 19 '21 17:05 danny-smit

@myii No, I don't have permission to add commits.

I've been chatting to @javierbertoli -- we're thinking about rebuilding the master images to the last commit before this problem started, keeping it there until there's some clarity from the Salt devs. If we do that, we can simply rerun the CI on your PRs and they should just work.

myii avatar May 19 '21 17:05 myii

@danny-smit So we've started fixing the images and most are working now. You can see the CI re-run of #289 here:

  • https://github.com/saltstack-formulas/docker-formula/pull/289/checks

It's actually wider-reaching than just the master builds. It's actually all the builds installing Salt using git (i.e. pip), so I'm in the process of fixing those as well.

myii avatar May 19 '21 20:05 myii

@myii Nice work. The tests of the CI re-runs are working much better now!

danny-smit avatar May 20 '21 11:05 danny-smit