add support for reading .j2 config files with jinja

[aelanman]

No description provided.

[andrerenard]

I think we should add jinja2 to the requirements.txt file.

[aelanman]

@andrerenard I've updated the requirements and marked this PR ready for review

[jrs65]

Is it worth trying to centralise the parsing of Jinja files into configs that kotekan will read? It would be nice to avoid the issue of having potentially divergent schemes for doing this parsing.

This would require solving the issue that kotekan simply embeds its processing into kotekan.cpp.

[andrerenard]

Perhaps this is a worth a wider discussion on the call this week, but I'm hesitant move to a model which calls python first to parse things and then calls kotekan. It could be done, but it creates some potential issues around how logging and debuggers/GPU profilers work. In most production situations kotekan runs as a service and it really has no need for python, and there is no reason to run it through a wrapper. So I could see being in a world where we end up with different files to run in different modes. If we just want to make sure we are parsing the same way (not that we are doing anything fancy yet) we could just call the python/scripts/config_to_json.py script in the kotekan repo which is the source for the kotekan.cpp call.

[andrerenard]

Notwithstanding @jrs65's concerns about centralizing the parsing logic, this looks good to me. However, I think we should try to get the aioredis_v2 branch merged into main before we merge this, since the way the CI is configured we aren't running the new unit test here. And it would be nice to keep the aioredis changes as a separate PR from this.

[jrs65]

If we just want to make sure we are parsing the same way (not that we are doing anything fancy yet) we could just call the python/scripts/config_to_json.py script in the kotekan repo which is the source for the kotekan.cpp call.

I would suggest doing this:

• Create a python module as part of the kotekan python package that has routines for doing all the parsing.
• Adding a if __name__ == "__main__" block to it such that it runs directly as a script without needing the package to be installed.
• Have coco install the kotekan python package and use the API from the new parsing module.
• Have kotekan either run the python script from an installed location in the fs, or have CMake embed the script at build time in a .h file, and have kotekan compile the string version of the script into the binary from that file (but otherwise run as it currently does).

I think that can be made to conveniently work in all envisaged cases from a single implementation.

However, we might want to pause on merging this PR until we've figured out what to do as in the case above the logic ends up exclusively in the kotekan python package.

[jrs65]

Agree. That we should merge the aioredis branch first. Currently it fails a bunch of the linting and doc building tests though.

[andrerenard]

@jrs65 I really like that proposal, and I'll also vote we implement this as a python package in the kotekan repo. That also makes it easier and more consistent for projects that want to use something other than coco for system control.

[andrerenard]

Changing this to "changes needed", in response to @jrs65's proposal, which I think we should implement.

[aelanman]

Agree. That we should merge the aioredis branch first. Currently it fails a bunch of the linting and doc building tests though.

@jrs65 It looks like none of these linting errors on that PR are from my changes. Should I still address them all?

[aelanman]

@andrerenard @jrs65 I've adjusted this accordingly with the changes in kotekan. I set the requirements.txt file to install the version of kotekan on the ael/jinja branch so the tests could pass, but I'll need to set that back to develop once that branch is merged.
kotekan/kotekan#1030

[jrs65]

@aelanman if you update this now #242 is merged I can take another look.

[aelanman]

@andrerenard @jrs65 I thought this was merged earlier today, but was mistaken. Is there anything else needed for this to be merged?

changes have been made