Import YAML with context #311
Conversation
There was a problem hiding this comment.
You need to amend your commit message for lint job to be satisfied. Please take a look here: https://github.com/conventional-changelog/commitlint/#what-is-commitlint
Other than that and one comment below, this looks good. Thanks for contributing!
postgres/map.jinja
Outdated
| {% import_yaml "postgres/osfamilymap.yaml" as osfamilymap %} | ||
| {% import_yaml "postgres/osmap.yaml" as osmap %} | ||
| {% import_yaml "postgres/codenamemap.yaml" as oscodenamemap %} | ||
| {% import_yaml "postgres/defaults.yaml" as defaults with context %} |
There was a problem hiding this comment.
The defaults mapping should not have any dynamic calls, so I think you'd better omit with context for it.
The warning message would be legitimate in this case.
There was a problem hiding this comment.
The defaults mapping should not have any dynamic calls, so I think you'd better omit
with contextfor it.
The warning message would be legitimate in this case.
I hit a similar issue a few of days ago, while merging a PR for the rspamd-formula. While all tests passed correctly on my dev env, they failed with the same error you describe on the saltstack-formula's env.
The only difference I saw is that the latter uses saltstack's master branch code while I was using packages-based images to test locally. I could reproduce the error using master.
Today, @myii directed me to a new saltstack's issue reported, which mentions that these errors are caused by a new jinja2 release and that salt is not playing nice with it. Maybe this is the root cause of these issues?
There was a problem hiding this comment.
Yeah that is the root cause, this could be considered as a temp fix until that is fixed. I encounter this to multiple salt formulas while updating to Salt v3003. So should we wait for the root cause fix?
There was a problem hiding this comment.
Yeah that is the root cause, this could be considered as a temp fix until that is fixed. I encounter this to multiple salt formulas while updating to Salt v3003. So should we wait for the root cause fix?
@Lucianovici Thanks for your PRs that are proposing fixes for this situation.
As I mentioned in that issue, this is affecting (at least) 32 of our formulas. Personally, I'd like to get some response from the Salt devs before making a final decision about this. I'd prefer not to "fix" all the formulas and then have to undo (or redo) the fixes again when that issue is resolved.
There was a problem hiding this comment.
A little update: in the meantime, while we wait for a response from the Salt devs, @javierbertoli and myself have added a workaround to the pre-salted images so that Jinja is downgraded from 3.0.x back to 2.11.3. This will allow our CI to keep working across all formulas for the time being.
Various discussions about the underlying issue: * saltstack/salt#60188 * saltstack-formulas/postgres-formula#311 * saltstack-formulas/docker-formula#290
Import yaml with context to avoid errors like: