search

audeering / audfactory

Communicating with Artifactory
https://audeering.github.io/audfactory/
Other
0 stars 0 forks source link

Ignore empty group_id #25

Closed frankenjoe closed 3 years ago

frankenjoe commented 3 years ago

With group_id='' we get a malformed URL:

audfactory.url(
    'https://host',
    group_id='',
    name='name',
    repository='repo',
    version='1.0.0',
)
https://host/repo//name/1.0.0

I guess it's better if we ignore group_id in that case.

hagenw commented 3 years ago

It's not directly malformed, but it might be that some programs cannot work with it.

hagenw commented 3 years ago

I agree, that we should change it.

hagenw commented 3 years ago

The same happens for name, repo.

For version it would add an / at the end, which would not happen otherwise.

hagenw commented 3 years ago

On the other hand you could also argue that it is a feature not a bug that group_id='' returns something different than group_id=None.

frankenjoe commented 3 years ago

I encountered the problem in audbackend at this line:

https://github.com/audeering/audbackend/blob/master/audbackend/core/artifactory.py#L41

when audfactory.path_to_group_id(folder) returns '' if folder is empty. So if we don't fix this here, we have to fix it in audbackend. But maybe we do similar things in other projects, so to me it feels safer to fix it here. Unless there is a use-case where we want to create an URL that has // in the path. But I doubt there is one. At least with Artifactory it will not find the artifact in that case.

hagenw commented 3 years ago

OK, as Artifactory cannot cope with it, I would say we should fix it here.