search

singularityhub / sregistry-cli

Singularity Global Client for container management
https://singularityhub.github.io/sregistry-cli/
Mozilla Public License 2.0
14 stars 18 forks source link

dateutils real requirement? #197

Closed tschoonj closed 5 years ago

tschoonj commented 5 years ago

I am looking into creating a conda package for sregistry with as many options supported as possible and I ran into trouble with the dateutils package, which at least according to its conda-forge package maintainers appears not to support python 3.

I was wondering if this package is really a dependency anyway as I cannot find any usage of it in the codebase, including the dateadd and datediff scripts that it installs.

If you agree that it can be removed from the dependency list, I will gladly open a PR to do this.

vsoch commented 5 years ago

It’s used for the Singularity Registry Server client to generate a time stamp for the authentication header.

vsoch commented 5 years ago

https://github.com/singularityhub/sregistry-cli/blob/master/sregistry/main/registry/utils.py

tschoonj commented 5 years ago

Aren't you using the datetime package for that?

vsoch commented 5 years ago

For Python 2.7, timezone wasn't a part of datetime - see this issue https://github.com/singularityhub/sregistry-cli/issues/44. What we could do is have support for conda for sregistry, and do a customized check to see if the Python version is 3.*. I'd be open to a PR for that! Would that work?

tschoonj commented 5 years ago

Well the way it is now, even with dateutils installed, it wouldn't be used anyways.

Honestly I wouldn't bother with it, and just stop supporting 2.7. In fact I assumed that this project was already python 3 only 😄 This is not a problem for Conda, as I was going to mark it as python 3 only.

Using dateutils.timezone for 2.7 would not be a trivial patch as datetime.timezone and dateutils.timezone are very different things (class vs method).

vsoch commented 5 years ago

Yeah I'm not a big fan of 2.7 anymore these days, it's definitely deprecated at this point.

What we could do is remove the dependency and then have a note in the README about the last version supporting 2.7, and then leave a gap in versions available in case we need to update an older version. There is a bit of repo admin work required here so I'm happy to do this.

tschoonj commented 5 years ago

Sure sounds good. Thanks for making the necessary changes!

Will open a PR soon for something else, as I found a problem in the S3.Client class.

vsoch commented 5 years ago

awesome!

here is a quick update to remove dateutils, and I've also done an official release for the last version with it, and added a note to the README about the release and branch. Could you take a look? https://github.com/singularityhub/sregistry-cli/pull/198