-
-
Notifications
You must be signed in to change notification settings - Fork 27
Introducing version_naming property that makes modulefile as version … #679
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
Changes from 8 commits
53e085f
e1996fa
5d60af3
2199f20
75240b5
4133ee7
9483d2b
5e0b994
6a47294
cc4be9b
a929ac0
52ae600
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -458,7 +458,10 @@ def install( | |
module.load_override_file() | ||
|
||
# Create the module and container directory | ||
utils.mkdirp([module.module_dir, module.container_dir]) | ||
if self.settings.version_naming: | ||
utils.mkdirp([os.path.dirname(module.module_dir), module.container_dir]) | ||
else: | ||
utils.mkdirp([module.module_dir, module.container_dir]) | ||
|
||
# Add a .version file to indicate the level of versioning | ||
self.versionfile.write( | ||
|
@@ -470,7 +473,14 @@ def install( | |
|
||
# Get the template based on the module and container type | ||
template = self.template.load(self.templatefile) | ||
module_path = os.path.join(module.module_dir, self.modulefile) | ||
if self.settings.version_naming: | ||
self._modulefile = name.split(":")[1] | ||
module._module_dir = os.path.dirname(module.module_dir) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please walk me through why we need a private class variable here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, forgot to clean it up, its not needed and have been fixed in new commit |
||
module_path = os.path.join( | ||
os.path.dirname(module.module_dir), self._modulefile | ||
) | ||
else: | ||
module_path = os.path.join(module.module_dir, self.modulefile) | ||
|
||
# Install the container | ||
# This could be simplified to take the module | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -68,6 +68,9 @@ singularity_shell: /bin/sh | |
podman_shell: /bin/sh | ||
docker_shell: /bin/sh | ||
|
||
#Experimental version naming tag so that modulefile is changed to version number . Default value is false. Issue number 26 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest having this right after |
||
version_naming: false | ||
|
||
# shell for test.sh file | ||
test_shell: /bin/bash | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,100 @@ | ||
#!/usr/bin/python | ||
|
||
# Copyright (C) 2021-2023 Vanessa Sochat. | ||
|
||
# This Source Code Form is subject to the terms of the | ||
# Mozilla Public License, v. 2.0. If a copy of the MPL was not distributed | ||
# with this file, You can obtain one at http://mozilla.org/MPL/2.0/. | ||
|
||
import io | ||
import os | ||
import shutil | ||
|
||
import pytest | ||
|
||
import shpc.main.registry as registry | ||
import shpc.utils | ||
|
||
from .helpers import here, init_client | ||
|
||
|
||
@pytest.mark.parametrize( | ||
"module_sys,module_file,container_tech", | ||
[ | ||
("lmod", "module.lua", "singularity"), | ||
("lmod", "module.lua", "podman"), | ||
("tcl", "module.tcl", "singularity"), | ||
("tcl", "module.tcl", "podman"), | ||
("lmod", "module.lua", "singularity"), | ||
("lmod", "module.lua", "podman"), | ||
("tcl", "module.tcl", "singularity"), | ||
( | ||
"tcl", | ||
"module.tcl", | ||
"podman", | ||
), | ||
], | ||
) | ||
def test_version_naming_install(tmp_path, module_sys, module_file, container_tech): | ||
|
||
""" | ||
Test that version naming install makes module file as version number | ||
""" | ||
client = init_client(str(tmp_path), module_sys, container_tech) | ||
|
||
client.settings.set("version_naming", True) | ||
assert client.settings.get("version_naming") == True | ||
# Install known tag | ||
client.install("python:3.9.2-alpine") | ||
|
||
# Modules folder is created | ||
assert os.path.exists(client.settings.module_base) | ||
|
||
module_dir = os.path.join(client.settings.module_base, "python") | ||
|
||
assert os.path.exists(module_dir) | ||
|
||
module_file = os.path.join(module_dir, "3.9.2-alpine") | ||
|
||
assert os.path.exists(module_file) | ||
|
||
@pytest.mark.parametrize( | ||
"module_sys,module_file,container_tech", | ||
[ | ||
("lmod", "module.lua", "singularity"), | ||
("lmod", "module.lua", "podman"), | ||
("tcl", "module.tcl", "singularity"), | ||
("tcl", "module.tcl", "podman"), | ||
("lmod", "module.lua", "singularity"), | ||
("lmod", "module.lua", "podman"), | ||
("tcl", "module.tcl", "singularity"), | ||
( | ||
"tcl", | ||
"module.tcl", | ||
"podman", | ||
), | ||
], | ||
) | ||
|
||
def test_disabled_version_naming_install( | ||
tmp_path, module_sys, module_file, container_tech | ||
): | ||
|
||
""" | ||
Test that version naming install makes module file as version number | ||
""" | ||
client = init_client(str(tmp_path), module_sys, container_tech) | ||
|
||
client.settings.set("version_naming", False) | ||
assert client.settings.get("version_naming") == False | ||
vsoch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# Install known tag | ||
client.install("python:3.9.2-alpine") | ||
|
||
# Modules folder is created | ||
assert os.path.exists(client.settings.module_base) | ||
|
||
module_dir = os.path.join(client.settings.module_base, "python", "3.9.2-alpine") | ||
|
||
assert os.path.exists(module_dir) | ||
module_file = os.path.join(module_dir, module_file) | ||
assert os.path.exists(module_file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is
self._modulefile
used for? I only see it in this small set of statements. It might make sense to just usename.split(":")[1]
in the one place it is needed and not save a private class variable.