-
-
Notifications
You must be signed in to change notification settings - Fork 31
spython.main.Client.build: add sudo_options option #140
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
Conversation
This allows me to pass '--preserve-env=SINGULARITY_CACHEDIR,SINGULARITY_TMPDIR' to sudo, which I need to deal with my very small /root and /tmp partitions.
|
I think we are forgetting one important thing... looks at CHANGELOG.md and version.py... |
|
Pushed that commit just now... |
|
Nope that is it! I think this is definitely needed, and it's good to handle the positioning and logic of adding the arguments with the base functions. Otherwise, we'd have to do the same checks (is the user using sudo? Are there sudo args?) in all the calling functions, and it would be redundant. So - it's a choice between having the lowest level functions (stream and run command) to have specialized options, or having redundant code. I like the first approach better, and based on your PR, I see that you do too. |
|
Yeah, I kind of went down the rabbit hole until I found where the Popen magic happens 🐰 |
|
By the way, your Circle-CI Orb setup is working really nicely for me... Thanks for that! I will now be using One thing I was wondering about... Do you think it would be a good idea to add the singularity installation to the cache as well? Now it is getting rebuilt every single time. Not hugely time consuming but still... |
|
That would be a great idea! I hadn't tackled it yet because I was worried about how to install the different versions alongside one another while still using a cache (for example, I've had Python build setups where I was caching Anaconda, and I thought I was testing Python 2 and 3, but in fact was just testing one, and the solution was to move them to different paths in the same cache. I believe there is an issue open for this #126 - do you want to either take a shot, or chat about how we could go about it (and then I'll try?) We would want to discuss if the cache should happen here, or with the Orb, and then how to properly do it. Anyway this PR is good to go - merging here and let's pick up conversation in #126! |
This allows me to pass '--preserve-env=SINGULARITY_CACHEDIR,SINGULARITY_TMPDIR' to sudo,
which I need to deal with my very small /root and /tmp partitions.