Document and refactor jinja2 macros for consistency and modularity#203
Document and refactor jinja2 macros for consistency and modularity#203bschwedler merged 20 commits intomainfrom
Conversation
Test Results646 tests 646 ✅ 3m 6s ⏱️ Results for commit f3bf649. ♻️ This comment has been updated with latest results. |
bschwedler
left a comment
bschwedler
left a comment
There was a problem hiding this comment.
The comments on the python file apply to the other dependencies.
I'm on the fence about which approach is better:
- More smarts in the macros, more compact Containerfile template, less end-user flexibility
- Call more macros directly, larger Containerfile template, more end-user flexibility
| {%- if packages -%} | ||
| {{ install_packages_from_list(packages, False) }}{% if files or clean %} && \{% endif %} | ||
| {%- endif -%} | ||
| {%- if files -%} | ||
| {%- for file in files -%} | ||
| {{ install_packages_from_file(file | trim, clean) }}{% if (not loop.last) or clean %} && \{% endif %} | ||
| {%- endfor -%} | ||
| {%- endif -%} |
There was a problem hiding this comment.
I like this approach It condenses a lot of the work down into a single layer.
| install packages from. Optional if packages is provided. | ||
| :param clean: Whether to remove the requirements.txt file(s) after installation. Defaults to True. | ||
| #} | ||
| {% macro install_packages(version, packages = None, requirements_files = None, clean = True) -%} |
There was a problem hiding this comment.
Do you think it would be useful to have a combined step that would install the dependency version and then install the packages, even if they are defined in separate layers?
It would just be fewer lines in the Containerfile template, but is more indirection.
There was a problem hiding this comment.
I broke them out because I think caching works better if we install the version independently of any packages. Packages will change frequently while the version install should remain the same, right?
… get around uv venv requirement
bschwedler
left a comment
bschwedler
left a comment
There was a problem hiding this comment.
Thank you sooo much for doing this!
All comments are generally about style, not substance.
2 participants
No description provided.