singularityhub / singularity-cli

streamlined singularity python client (spython) for singularity
https://singularityhub.github.io/singularity-cli/
Mozilla Public License 2.0
57 stars 31 forks source link

Do you have plans to support version 3 of singularity? #75

Closed al3x609 closed 5 years ago

al3x609 commented 5 years ago

...

vsoch commented 5 years ago

If Singularity is consistent and reproducible, the upgrade (other than new features) should still have most working here. Are there specific issues you are running into?

al3x609 commented 5 years ago

no problem; I'm just testing version 3. *, but I do not find apparent improvement in performance, rather than signed images.

The only inconvenience of compatibility that I have found, is when using instances, due to the separation of sub commands. For example $ singularity instance.list vs $ singularity instance list

vsoch commented 5 years ago

oh I see, so this is for the command line, e.g.,

spython instance list

You would rather have:

spython instance.list

?

al3x609 commented 5 years ago

yes, but it is not so necessary, I prefer to use version 2, while the 3 becomes more stable and find more cases of usability to the new features. Thank you 👍

vsoch commented 5 years ago

okey doke, and to answer your question, yes I do plan to support the needs / requests of the users of Singularity. So if you want a change / update, just let me know and we will work on it. :)

al3x609 commented 5 years ago

ok, maybe the instance command group compatibility, not only for command line, if not also when using python as API

e.g. https://github.com/singularityhub/singularity-cli/blob/9dfef9dea7e5fe3489e8ff44f66896823819a82c/spython/main/instances.py#L45

https://github.com/singularityhub/singularity-cli/blob/9dfef9dea7e5fe3489e8ff44f66896823819a82c/spython/instance/cmd/start.py#L55

I would appreciate it, this compatibility.

vsoch commented 5 years ago

Hmm so you are saying Singularity 3.0 has changed "instance.list" to "instance list" ?

vsoch commented 5 years ago

For older versions, the instance command subgroup corresponds with what is implemented now

$ singularity --version
2.5.2-dist
$ singularity instance.   # pressing tab here
instance.list   instance.start  instance.stop  
al3x609 commented 5 years ago

Hmm so you are saying Singularity 3.0 has changed "instance.list" to "instance list" ?

yes, Replaced singularity instance. command group with singularity instance , take it from changelog

vsoch commented 5 years ago

ah thank you! Yes I'd be happy to add this, I'd add support for both. I'll likely have some time tomorrow to get a PR for you to test!

al3x609 commented 5 years ago

thanks for your time :) Im working with singularity version 3.0.1-71.g9b2e243 from GITHUB repo

vsoch commented 5 years ago

@al3x609 ready for your review https://github.com/singularityhub/singularity-cli/pull/76

al3x609 commented 5 years ago

Hi, the mod fail, because the command is not well structured.

I put a test code in some modules.


full log

In [11]: inst = cl.instance("/data/singularity/containers/base.1.0.sif")
singularity version 3.0.1-71.g9b2e243
['singularity', ['instance.start'], '/data/singularity/containers/base.1.0.sif', 'lovable_knife_1589']
<class 'list'>
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
/usr/local/lib/python3.7/site-packages/spython-0.0.47-py3.7.egg/spython/client/shell.py in <module>()
----> 1 inst = cl.instance("/data/singularity/containers/base.1.0.sif")

/usr/local/lib/python3.7/site-packages/spython-0.0.47-py3.7.egg/spython/instance/__init__.py in __init__(self, image, start, **kwargs)
     41        # Start the instance
     42        if start is True:
---> 43            self._start(**kwargs)
     44
     45 # Unique resource identifier

/usr/local/lib/python3.7/site-packages/spython-0.0.47-py3.7.egg/spython/instance/cmd/start.py in start(self, image, name, sudo, options, capture)
     76     self.cmd = cmd
     77
---> 78     output = run_command(cmd, sudo=sudo, quiet=True, capture=capture)
     79
     80     if output['return_code'] == 0:

/usr/local/lib/python3.7/site-packages/spython-0.0.47-py3.7.egg/spython/utils/terminal.py in run_command(cmd, sudo, capture, no_newline_regexp, quiet)
    143     process = subprocess.Popen(cmd,
    144                                stderr = subprocess.PIPE,
--> 145                                stdout = stdout)
    146     lines = ()
    147     found_match = False

/usr/local/lib/python3.7/subprocess.py in __init__(self, args, bufsize, executable, stdin, stdout, stderr, preexec_fn, close_fds, shell, cwd, env, universal_newlines, startupinfo, creationflags, restore_signals, start_new_session, pass_fds, encoding, errors, text)
    754                                 c2pread, c2pwrite,
    755                                 errread, errwrite,
--> 756                                 restore_signals, start_new_session)
    757         except:
    758             # Cleanup if the child failed starting.

/usr/local/lib/python3.7/subprocess.py in _execute_child(self, args, executable, preexec_fn, close_fds, pass_fds, cwd, env, startupinfo, creationflags, shell, p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite, restore_signals, start_new_session)
   1428                             errread, errwrite,
   1429                             errpipe_read, errpipe_write,
-> 1430                             restore_signals, start_new_session, preexec_fn)
   1431                     self._child_created = True
   1432                 finally:

TypeError: expected str, bytes or os.PathLike object, not list

cmd = ['singularity', ['instance.start'], '/data/singularity/containers/base.1.0.sif', 'lovable_knife_1589']

return from https://github.com/singularityhub/singularity-cli/blob/bd5f6b7ede7d2baaf3e17ee31466ea841f00ed6b/spython/instance/cmd/start.py#L59

singularity version 3.0.1-71.g9b2e243

never start with 3, always take version=2

maybe fix

if some_string.find("version 3"):
    subgroup = ["instance", "start"]

or something more pythonic :) ,

vsoch commented 5 years ago

Just a typo / bug! Try again.

al3x609 commented 5 years ago

the log IndexError: list index out of range


not, You did not understand me,

singularity --versionin singularity **2.*** return:

$ singularity --version
2.5.2-dist

singularity --version in singularity **3.*** return:

$ singularity --version
singularity version 3.0.1-71.g9b2e243

the error is not from the API, but singularity does not return the same type of value.

vsoch commented 5 years ago

I'm parsing from the command line, it would always be a string.

vsoch commented 5 years ago

but I see the difference, I'll try remove singularity version from the second.

vsoch commented 5 years ago

okay, give another try! Please keep up the condescending italics and curt patronizing statements, it's really so kind of you! Here, have some :popcorn: for a munchie.

al3x609 commented 5 years ago

I'll keep it in mind thanks, it works, 👍 I also made an issue request to Singularity repo.

report

vsoch commented 5 years ago

Great thanks! I'll merge the PR and have the updated version on pypi in a few minutes after. Thanks for the catch!

vsoch commented 5 years ago

Released: https://pypi.org/project/spython/0.0.47/

al3x609 commented 5 years ago

thanks again, :) Vanesa