Skip to content
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

Client.instance(image, name) :: change name when it contains '-' #215

Open
EricDeveaud opened this issue Dec 11, 2023 · 4 comments
Open

Comments

@EricDeveaud
Copy link
Contributor

EricDeveaud commented Dec 11, 2023

Hello,
when one instanciate an instance giving a name that contains a - (minus sign).
the generated instance have a name slightly changed to name whit - (minus) replaced by _ underscore. see:

>>> Client.instance('/my/image.sif', name='foo-1.2.3')
instance://foo_1.2.3

as far I as understand this is due to: the call of generate_name in spython/instance/__init__.py

    def generate_name(self, name=None):
        """generate a Robot Name for the instance to use, if the user doesn't
        supply one.
        """
        # If no name provided, use robot name
        if name is None:
            name = self.RobotNamer.generate()
        self.name = name.replace("-", "_")

I was expecting to have an instance with name according to the one provided

is there a reason, that I don't catch, for this change

regards

Eric

@vsoch
Copy link
Member

vsoch commented Dec 11, 2023

We don’t allow the dashes and use underscores instead. It’s just a convention.

@EricDeveaud
Copy link
Contributor Author

hummm

ok I will adapt to this convention.
NB in my case (packaging stuff) having instances with the name of the tools I'm packaging is helpfull
eg

blast-2.14.1
hmmer-3.4

anyway I can live with that
or just comment the replace line in my own spython copy ;-)

Eric

@vsoch
Copy link
Member

vsoch commented Dec 11, 2023

I would be OK with matching the convention that SIngularityCE uses - if they allow dashes (and have a regex to check) we can do that too. I'm open to reviewing a PR that makes this change if you'd like it!

@EricDeveaud
Copy link
Contributor Author

look like dash is allowed.

when checking singularity instance_linux.go you can see that name must comply with authorizedChars reg-exp that is defined this way.
authorizedChars = ^[a-zA-Z0-9._-]+$

PR submited.

regards

Eric

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants