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

Properly declare dependency on zope.schema >= 4.2.0 #63

Merged
merged 1 commit into from
Oct 25, 2019

Conversation

cjwatson
Copy link
Contributor

This was introduced in zope.security 4.2.1.

This was introduced in zope.security 4.2.1.
@cjwatson
Copy link
Contributor Author

I happened to run into this when upgrading Launchpad's dependencies from fairly old versions, getting a traceback along the way that ended in:

Traceback (most recent call last):
...
  File "/home/cjwatson/src/canonical/launchpad/git/launchpad/env/local/lib/python2.7/site-packages/zope/security/interfaces.py", line 56, in <module>
    from zope.schema import Text, TextLine, NativeStringLine
ImportError: cannot import name NativeStringLine

It would be a bit friendlier to declare this.

Copy link
Member

@jamadden jamadden left a comment

Choose a reason for hiding this comment

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

LGTM.

It looks like I originally introduced the import of NativeStringLine without bumping the dependency version a couple years back. zope.schema 4.2 was around four years old at that point so I guess it didn't even occur to me that adding a version pin would be helpful.

Existential thoughts that can be ignored: zope.schema is at version 4.9.3 now, and that's what CI is testing, so it occurs to me to wonder if 4.2 is a sufficient enough pin. Most likely it will rarely be tested in that exact configuration. There's a tension between end-user ease of deployment and ease of development here. It's a hard problem. None of this is new of course. Thanks for fixing my error! I'm sorry you had to deal with it.

Copy link
Member

@mgedmin mgedmin left a comment

Choose a reason for hiding this comment

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

Thank you for caring!

@cjwatson
Copy link
Contributor Author

Yeah, I don't have a better solution than either (a) bump versions when we notice problems or (b) a huge tox matrix to test the minimum version pins. Probably not worth the effort for (b) ...

@cjwatson cjwatson merged commit a874757 into zopefoundation:master Oct 25, 2019
@cjwatson cjwatson deleted the declare-zope.schema-4.2.0 branch October 25, 2019 08:42
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

Successfully merging this pull request may close these issues.

3 participants