Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Creation of envfile to be made conditional? #406

Open
marcodelapierre opened this issue Jun 11, 2021 · 5 comments
Open

Creation of envfile to be made conditional? #406

marcodelapierre opened this issue Jun 11, 2021 · 5 comments
Labels
more-input-needed This label indicates that this issue needs more discussion or +1/-1 to decide how and if to continue

Comments

@marcodelapierre
Copy link
Contributor

One minor, last thought I had while scrolling through the current setup for envfile.

Always having the --envfile "..." in the modulefile, even when it's not used, makes it quite less readable.
I was then wondering whether it should be made conditional?

...Actually I see it's already conditional in the modulefile templates: {% if envfile %}.
So it's just about ensuring that envfile is set to null if no envs are defined in the container yaml?

main/container/singularity.py:            envfile=self.settings.environment_file,
main/container/docker.py:            envfile=self.settings.environment_file,
@vsoch
Copy link
Member

vsoch commented Jun 11, 2021

My thinking for this was that (for the most part) I think people would be likely to want to add envars after install - if the default in settings always generates the file, that's easy to do - just edit the file! Now for your case if you want to just nix the entire thing, I think you would just set the settings environment file to be null.

@marcodelapierre
Copy link
Contributor Author

Ah I see!
Of course this is subjective :)

My thought was in the spirit of using almost only SHPC to handle the setup of Container Modules.
So users willing to add envvars would define them in the recipe, then (re-)install with SHPC.

But if you think that some people would prefer to add envars independently from SHPC, then it makes complete sense - and ignore this Issue :-)

@marcodelapierre
Copy link
Contributor Author

And actually I agree that your current solution is more flexible from a user perspective!

@vsoch
Copy link
Member

vsoch commented Jun 11, 2021

I figure it doesn't hurt to have, but it could hurt to not. But let's leave this issue open for future users to comment on!

@marcodelapierre
Copy link
Contributor Author

agree!

@vsoch vsoch added the more-input-needed This label indicates that this issue needs more discussion or +1/-1 to decide how and if to continue label Jun 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
more-input-needed This label indicates that this issue needs more discussion or +1/-1 to decide how and if to continue
Projects
None yet
Development

No branches or pull requests

2 participants