Skip to content

refactoring: replacing singularity with apptainer in variable names #1366

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

MontrealSergiy
Copy link
Contributor

resolves #1362

@MontrealSergiy MontrealSergiy self-assigned this Dec 18, 2023
@MontrealSergiy MontrealSergiy linked an issue Dec 18, 2023 that may be closed by this pull request
@MontrealSergiy MontrealSergiy linked an issue Dec 18, 2023 that may be closed by this pull request
@MontrealSergiy MontrealSergiy changed the title refactoring: replacing singularity with apptainer in variable name refactoring: replacing singularity with apptainer in variable names Dec 18, 2023
Copy link
Member

@prioux prioux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments

Copy link
Member

@prioux prioux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This first thing to do is to rebase and make sure the database changes are applied on top of the schema that the current head of master implements.

After that, some minor renaming comments.

I would remove all the "Apptainer/Singularity" comments and just say "Apptainer".

@@ -206,7 +206,7 @@ def update #:nodoc:
@tool_config.container_image_userfile_id = other_tc.container_image_userfile_id
@tool_config.container_exec_args = other_tc.container_exec_args.presence
@tool_config.container_index_location = other_tc.container_index_location
@tool_config.singularity_overlays_specs = other_tc.singularity_overlays_specs
@tool_config.apptainer_overlays_specs = other_tc.apptainer_overlays_specs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation please?

@@ -424,15 +424,15 @@ def make_available(userfile, file_path, userfile_sub_path = nil, start_dir = nil
userfile = Userfile.find(userfile) unless userfile.is_a?(Userfile)

# Detect if we're trying to access a userfile content that is
# containerized in a singularity overlay.
# containerized in a apptainer overlay.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"an Apptainer" ; use uppercase when talking about the program itself.

is_local = nil
if self.tool_config
.data_providers_with_overlays
.map(&:id)
.include? userfile.data_provider_id
userfile_path = Pathname.new(userfile.provider_full_path) # path inside container
is_local = true
self.addlog("Input file '#{userfile.name}' is on a local Singularity overlay")
self.addlog("Input file '#{userfile.name}' is on a local apptainer overlay")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Apptainer"

@@ -618,10 +618,10 @@ def tool_config_system(command)

# Build script
script = ""
# flag to guaranty propagation of env variables to the singularity/apptainer
# flag to guaranty propagation of env variables to the singularity/Apptainer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove singularity

@@ -2406,26 +2406,26 @@ def singularity_commands(command_script)

# These two variables control the mode switching at the end of the script.
mode="exec"
sing_basename=./#{singularity_wrapper_basename.bash_escape} # note: the ./ is necessary
sing_basename=./#{apptainer_wrapper_basename.bash_escape} # note: the ./ is necessary
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also rename the shell variable "sing_basename" ? Maybe 'apptainer_basename' ?

@@ -22,22 +22,22 @@

# This class implements a Data Provider which fetches
# files out of a single filesystem file. The
# implementation requires Singularity 3.2 or better to
# be installed on the host, as well as a singularity
# implementation requires Apptainer 1.1 or Singularity 3.7 and better
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to mention singularity

@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 20230418205141) do
ActiveRecord::Schema.define(version: 20240408164549) do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THis PR will ahev to be rebased and the version numbers edited to match the current version

@prioux
Copy link
Member

prioux commented Mar 20, 2025

Please rebase and address the comments.

@MontrealSergiy MontrealSergiy changed the base branch from dev to master April 3, 2025 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename all code that refers to 'Singularity' as 'Apptainer'
2 participants