Skip to content

Commit

Permalink
Add python_requires with 2.7.9 and 3.3 as minimum versions.
Browse files Browse the repository at this point in the history
  • Loading branch information
@mikedarcy
mikedarcy committed Oct 18, 2018
1 parent 0d0755a commit 86517b9
Showing 1 changed file with 14 additions and 2 deletions.
16 changes: 14 additions & 2 deletions setup.py
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
#
# Copyright 2015 University of Southern California
# Distributed under the Apache License, Version 2.0. See LICENSE for more info.
# Copyright 2016 University of Southern California
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

""" Installation script for the BDBag utilities.
Expand Down Expand Up @@ -65,6 +76,7 @@
'boto': ["boto3>=1.9.5", "botocore", "awscli"],
'globus': ["globus_sdk>=1.6.0"],
},
python_requires='>=2.7.9, !=3.0.*, !=3.1.*, !=3.2.*, <4',
license='Apache 2.0',
classifiers=[
'Intended Audience :: Science/Research',
Expand Down

9 comments on commit 86517b9

@nsoranzo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikedarcy Can you please give some detail about the >=2.7.9 Python requirement? We use bdbag in Galaxy and one of our major deployment platforms (CentOS 7) still uses python 2.7.5. Thanks!
We are probably going to pin bdbag to 1.4.1 in the mean time.

@mikedarcy
Copy link
Collaborator Author

@mikedarcy mikedarcy commented on 86517b9 Dec 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this requirement stems from this single line change in the released code of bagit-1.7.0 that the current release of bdbag is pinned to: LibraryOfCongress/bagit-python@54dee23#diff-70e40b055f80c9089d8e81b75e7fb9c1L118

In this commit, bagit-python dropped support for Python 2.6, but introduced (inadvertently?) a dependency on Python 2.7.9 as the minimum Python 2.7 version due to the inclusion of the direct use of hashlib.algorithms_guaranteed.

Because bdbag "monkeypatches" and overrides certain functions from bagit-python, we do a strict pin on the currently supported version in case a release of bagit-python changes code that we are overriding. We are currently pinned to bagit-1.7.0, because we want to support the latest stable version of bagit-python in bdbag.

Ideally, my preferred solution for this change would be to have it reverted in bagit-python so that older versions of Python 2.7 are supported, but I'm not too sure how realistic that is. Alternatively, it is possible to just install the latest version of bdbag from source, use bagit-1.6.4 as the installed bagit version, and remove the strict pin statement in bdbag's setup.py. This is of course not the ideal solution, but am fairly confident it will work.

I will investigate some other ways to get around this, but at the moment it is a bit of a conundrum. Its a shame that in the current state of things an entire OS distro cannot be used (without significant modification) with the latest bdbag/bagit because of one line of code.

@nsoranzo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikedarcy Thanks a lot for your reply and clarification, I've left a comment on https://github.com/LibraryOfCongress/bagit-python/pull/110/files#r238885995 , hopefully this can be resolved in bagit upstream.

@nsoranzo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've opened LibraryOfCongress/bagit-python#123

@mikedarcy
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for driving this @nsoranzo. If bagit-python generates a point release including this fix, I will generate a new bdbag release pinned to it and remove the 2.7.9 condition from python_requires.

@mikedarcy
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nsoranzo I've pushed some changes to master for the 1.5.2 point release that includes a "monkeypatch" for hashlib.algorithms_guaranteed when it is not found. The patch basically does the same thing as what you proposed here but patches hashlib.algorithms_guaranteed directly and before any bagit code is imported so that the attribute exists by the time the bagit code tries to dereference it. I tested it out on a quick AWS spin-up of a Ubuntu 14.04 LTS and it works fine. This approach won't make bagit-1.7.0 work standalone on such systems but will allow bdbag to function properly.

Would you mind testing this change out (just pull master) and letting me know what you think about this fix? If this is good enough for you, I will include it in the 1.5.2 point release and remove the strict pin on Python=>2.7.9.

@nsoranzo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this @mikedarcy !

I tested on Ubuntu 12.04 and all seems fine except for a small issue:

$ docker pull ubuntu:trusty
$ docker run -it ubuntu:trusty bash
root@548cd7420aae:/# apt update 
...
root@548cd7420aae:/# apt upgrade -y
...
root@548cd7420aae:/# apt install -y git python2.7 python-pip
...
root@548cd7420aae:/# python --version
Python 2.7.6
root@548cd7420aae:/# pip install --upgrade pip setuptools
...
root@33d2366d80de:/# pip install bdbag==1.5.1
...
root@33d2366d80de:/# python
Python 2.7.6 (default, Nov 13 2018, 12:45:42) 
[GCC 4.8.4] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import bdbag.bdbagit
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python2.7/dist-packages/bdbag/bdbagit.py", line 18, in <module>
    import bagit
  File "/usr/local/lib/python2.7/dist-packages/bagit.py", line 125, in <module>
    CHECKSUM_ALGOS = hashlib.algorithms_guaranteed
AttributeError: 'module' object has no attribute 'algorithms_guaranteed'
>>> 
root@33d2366d80de:/# pip install git+https://github.com/fair-research/bdbag
...
root@33d2366d80de:/# python
Python 2.7.6 (default, Nov 13 2018, 12:45:42) 
[GCC 4.8.4] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import bdbag.bdbagit
>>> import bdbag.fetch.resolvers.doi_resolver   
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/.venv/local/lib/python2.7/site-packages/bdbag/fetch/resolvers/doi_resolver.py", line 19, in <module>
    from bagit import CHECKSUM_ALGOS
  File "/.venv/local/lib/python2.7/site-packages/bagit.py", line 125, in <module>
    CHECKSUM_ALGOS = hashlib.algorithms_guaranteed
AttributeError: 'module' object has no attribute 'algorithms_guaranteed'
>>>

The latest issue is fixed by #30 .

@mikedarcy
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged #30. @nsoranzo So is this approach "good enough" for the time being? It should at least give us some functional cushion while we wait for bagit to fix the underlying issue and/or the official end-of-life for the distros in question.

@nsoranzo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikedarcy Thanks, that looks good enough for me too!

Please sign in to comment.