PR210, the introduction og `mig/__jinja__`

[rasmunk]

Initially I commented directly on this in PR210 #210 (comment), but since it might be more than a single line change I have pulled it out into this.

Has there been any discussion about the location of the cache inside the MIG_BASE/migrid/__jinja__?

Wondering if this is the best place for them. Feels like the existing state/mig_system_run would be a better place to put them and maybe the directory should describe the purpose such as primed_templates/jinja_cache, templates/jinja_cache or similar that also links it back to how the jinja cache was generated.

[albu-diku]

No discussion really - I don't know what's best. I put it somewhere that felt roughly reasonable.

One thing to note about the choice, it was meant to highlight that this isn't really changing data, hence choosing a path that would make it clearer it is basically static data (which hopefully will make sense after the discussion about priming we had).

[rasmunk]

Given that they are primed during the build and therefore persistent across runs unless the templates are changed, I would think that state/templates/jinja_cache is the best destination for them.

[albu-diku]

Doesn’t the state directory persist across builds? If so, following the suggestion will result in stale template bytecode were a template to ever be removed, unless we empty the directory but I can see us having other concerns with that.

It needs to be a folder persisted as part of the build but bot survive beyond that - so I don’t think state is the right location.

[rasmunk]

The state directories do persist over time, but since the primed templates are a compiled cache based on the template packages specified in the configuration they atleast in my mind reflect a state file based on the current configuration.

In terms of availability during build, we could make it work with the existing state directories albeit this can quickly become messy when overlay mounts come into the picture. Therefore have I started to reconsider whether it might no be better to delay the template compilation from the build stage, to the init phase when we start the migrid stack. That is to move the primeing to happen right before the httpd service is launched and they are needed, in for instance https://github.com/ucphhpc/docker-migrid/blob/c71118da77d56d8f9f75a4f7f35bf76ca9c495a2/docker-entry.sh#L162. This ties it directly to where it is needed ahead of the httpd service.

In terms of removing/updating templates, I would think it would be a good feature of the templates to be a good citzen, that is to provide functionality to cleanup after themselves, beit flushing old cache files and existing templates via for instance a combination of make targets or an mig/install/migrid-init.d-{deb,rh}-template init option. I would imagine that this will also be useful if we have to for instance replace existing templates during production/runtime manually or via hotfixes.

[albu-diku]

Ok.

So at the outset, I don't think changing this right now is really critical relative to getting the rest of the UI in and people (internally) using that version of the system. We can move this directory anywhere we want and we can call priming wherever we'd like, but I think we are falling into making this PR absolutely perfect up front when we ought to be focused on the larger picture. The priming process was written/exposed as a CLI precisely because it would make it malleable.

With that stated, it's fair to discuss something different here but what you're proposing quite substantially changes the intent. The provision of template packages happens at build time, and which templates are enabled is static (particularly given the current docker migrid which hard codes a configuration into the containers it produces). The associated bytecode is part of the build. I wanted that represented as part of the build process. In addition, I really do not want the bytecode for these things to have to live in a writeable directory period. Regardless of whether or not that directory ought to be user accessible I don't want to introduce such a risk.

In summary, I don't think changing this is bringing value and I think your suggestion creates other problems. I would rather just agree upon a better directory to use as part of the build.

[rasmunk]

It wasn't to create a huge rewrite or anything of the likes and I'm sorry if it gave that impression, I only see 3 small (atleast that is the intention) changes outstanding before we merge PR210. This and the two other minor things in the PR and we should be good to go on merging it. I also really like the foundation for the mig.lib.templates CLI, which as you say makes it easy to extend with future functionality such as templates cleanup which I agree with is not necessary at the outset but was more a future looking addition which I should have emphasized. The point was to add to the flexibility and ensure the future possibility being able to configure the system as required during scheduling/launch where possible (in addition to likely rare runtime hotfixes) like you would expect it to be able to, which I agree with is not how we do it right now, but hopefully we will be able to do.

But yes, we can move ahead with just a better directory and postpone any such changes/additions to after we have gotten the basic template infrastructure integrated. Maybe for now we go with a mig/templates/jinja_cache that we then don't mount as part of the docker-migrid deployments, @jonasbardino , @Martin-Rehr any inputs/apprehension on this?

[Martin-Rehr]

To my understanding jinja compiles templates lazily on first use and can cache the resulting bytecode for reuse via its built-in BytecodeCache. I'd suggest letting Jinja handle this at runtime storing the cache in state/mig_system_run/templates[1] rather than precompiling manually.

[1] state/mig_system_run is on tmpfs at our production systems

[albu-diku]

I am making the changes as requested, but it will involve touching all the PRs to remove the priming. The other consequence of doing so is that the single cache directory, within which the bytecode will be compiled, does still need to be created since it will be on temporary storage. I can't see any good place to add this during startup (unless we change both init.d and docker scripts), with the alternative being to cause every single request that makes use of templates to touch the filesystem to check the dir exists, something I had hoped to avoid.

Please let me know what you would like me to do.

[Martin-Rehr]

The creation of volatile cache directories are handled in the Makefile of docker-migrid:
https://github.com/ucphhpc/docker-migrid/blob/c71118da77d56d8f9f75a4f7f35bf76ca9c495a2/Makefile#L106

[albu-diku]

I need to create the subdirectory for the templates cache - it must exist for the bytecode to be written. Shall I add it to that long list?

[albu-diku]

Will place them a the top level so as to move forward.

[Martin-Rehr]

	mkdir -p ${VOLATILE_ROOT}/mig_system_run
	mkdir -p ${VOLATILE_ROOT}/openid_store
+   mkdir -p ${VOLATILE_ROOT}/templates

Would be my suggestion