[bug] We lost retrocompatibility with 2.* Singularity version.

[al3x609]

a few months ago, we made a few changes for support Singularity 3.* instance group commands,

i.e

singularity-cli/spython/instance/cmd/stop.py

Lines 31 to 34 in f84ef3e

	subgroup = 'instance.stop'
	if get_singularity_version().find("version 3"):
	subgroup = ["instance", "stop"]

but this break compatibility with 2.* branch, because always go inside the IF statement, it was my mistake, :( .

this can fix that.

if ( get_singularity_version().find("version 3") != -1 ):

now, testing singularity 3.* commands, they now support two ways to return singularity version

I think that they we listen to us :)

• singularity version
  3.1.0-rc4.dirty
• singularity --version
  singularity version 3.1.0-rc4.dirty

What you think about that?

Can this return value be used to prevent including unnecessary code?

[vsoch]

Okay for the first part:

but this break compatibility with 2.* branch, because always go inside the IF statement, it was my mistake, :( .

Can you give an example of why this would happen? Is it just a bug with how we used find? This is definitely easy to fix!

For the second part, for all of the versions you listed, why do some have "singularity version" and others not? And what do you mean by:

Can this return value be used to prevent including unnecessary code?

Oh so instead of checking the string, just check the return value (is that what you mean?) That's a great idea!

[al3x609]

ok, for the first:

e.g in ipython:
first test

in [1]: version='singularity version 3.1.0-rc4.dirty'
in [2] version.find("version 3")
out [3]: 12

If substring exists inside the string, it returns the index of first occurence of the substring.
If substring doesn't exist inside the string, it returns -1.

second test

in [1]: version='singularity version 3.1.0-rc4.dirty'
in [2] version.find("version 2")
out [3]: -1

now, no matter if the return value is a positive or negative, the IF statement always is executed, and the variable subgroup always will be replaced with ["instance", "stop"] ,

which gives an error in version 2. * of Singularity

i.e

if(4):
 print("test")

test
__________________________________________________________________________________________________________
if(-1):
 print("test")

test

It is something simple, but many times simple things are the least we consider, and we will have an almost unperceptible logic error.

for the second part:

it's just a comment to show you that they return two different strings when looking for the singularity version. which can trigger errors for other applications that require this value, in this case Spython.

from your comment

[vsoch]

Yep that's a bug! I'll start work on it now. I have a PR here for other instance tweaks, so I'll let you know when I've updated it (and if you could test the components involved, would be greatly appreciated).

Stay tuned!

[vsoch]

okay please test the PR. With respect to your observation, it's indeed the case that singularity --version returns a different result from singularity version. Since the flag was provided in the original command, and we have choice to use one or the other, we can have consistency in using it (and I don't see it being an issue).

[al3x609]

it works. your fix is more pythonic code :) , also i check the PR that returns an instance object instead of a List, and it works correctly.

with ipython, the next modules are tested.

• iutils.py
• start.py
• stop.py
• instances.py

---

one more thing, the docstring for Stop function needs to be updated.

singularity-cli/spython/instance/cmd/stop.py

Lines 12 to 20 in e6fcbaa

	def stop(self, name=None, sudo=False):
	'''start an instance. This is done by default when an instance is created.
	Parameters
	==========
	image: optionally, an image uri (if called as a command from Client)
	name: a name for the instance
	sudo: if the user wants to run the command with sudo

[vsoch]

Good call! Let me fix that quickly.

[vsoch]

okay, fixed! Take a look and let me know if it's ok now :)

[al3x609]

Oops, you forgot this line. :p

singularity-cli/spython/instance/cmd/stop.py

Line 13 in 8097f24

'''start an instance. This is done by default when an instance is created.