Update requirements to fix pip-compile issues #169
Conversation
|
Thanks for the pull request, @kaizoku! I've created OSPR-4113 to keep track of it in JIRA. JIRA is a place for product owners to prioritize feature reviews by the engineering development teams. Feel free to add as much of the following information to the ticket:
All technical communication about the code itself will still be done via the GitHub pull request interface. As a reminder, our process documentation is here. |
openedx-webhooks
added
needs triage
open-source-contribution
|
@kaizoku, it looks like the changes in this PR break CI checks, particularly the Python 2.7 ones. So we have to decide on supporting Python 2.7 (for the current and older releases) and if we decide not to, I think a major version bump for this library will be required to indicate a backward-incompatible change. @feanil, @jmbowman, as the reviewers of the recently merged PRs in this repo, can you check this out when you can and let us know your thoughts? |
requirements/dev.txt
Outdated
| markupsafe==1.1.1 # via jinja2, mako, xblock | ||
| mccabe==0.6.1 # via pylint | ||
| mock==3.0.5 | ||
| more-itertools==5.0.0 # via pytest, zipp | ||
| more-itertools==8.2.0 # via pytest |
requirements/dev.txt
Outdated
| astroid==1.6.6 # via pylint, pylint-celery | ||
| atomicwrites==1.3.0 # via pytest | ||
| appdirs==1.4.3 # via fs, virtualenv | ||
| astroid==2.3.3 # via pylint, pylint-celery |
|
We've started dropping Python 2.7 support from a number of repositories, figuring that most things should be on Python 3 by now and the few stragglers can stay pinned to older versions that still support 2.7. So I'm ok with dropping the Python 2.7 support, but that should probably be a PR of its own. To fix CI while retaining Python 2.7 support, you can add a |
|
@kaizoku Thank you for your contribution. Please let me know once this is ready for our review. |
openedx-webhooks
added
waiting on author
|
The tests are passing on Travis now, but the quality checks are failing. Looks like pydocstyle needs to be constrained to be lower than version 4.0.0 (which dropped Python 2 support). And there's one lint failure complaining about use of a deprecated method. |
1 similar comment
|
The tests are passing on Travis now, but the quality checks are failing. Looks like pydocstyle needs to be constrained to be lower than version 4.0.0 (which dropped Python 2 support). And there's one lint failure complaining about use of a deprecated method. |
|
Ah sorry I didn't check the quality on my local machine @jmbowman. I added the Python 3 documentation notes that "This method was previously documented as deprecated in favour of signature() in Python 3.5, but that decision has been reversed". Is it safe to add a pylint ignore deprecation for this function for now, given that it won't actually be deprecated? |
@kaizoku, 👍 It could also be useful to link to the relevant documentation to indicate why we are ignoring that warning. |
pylint warns about a deprecated function during quality checks. This function was meant to be deprecated, but python maintainers decided to reverse this decision. https://docs.python.org/3/library/inspect.html#inspect.getfullargspec
|
@lgp171188 thanks! I added the pylint ignore. |
|
@kaizoku 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
|
@jmbowman, thanks for reviewing and merging this PR. Can you tag a new release that includes the changes in this PR so that we can use that in open-craft/problem-builder#262? |
|
@jmbowman, did you get a chance to check my previous comment about tagging a new release? |
|
Sorry, I apparently missed the notification for your last message; done: https://github.com/edx/xblock-sdk/releases/tag/v0.1.8 |
| certifi==2019.11.28 # via requests | ||
| chardet==3.0.4 # via binaryornot, requests | ||
| cookiecutter==0.9.0 | ||
| django-pyfs==2.0 | ||
| django==1.11.26 | ||
| django==2.2.10 |
There was a problem hiding this comment.
@kaizoku Was this intended? django 2.2.10 isn't supported on python2. Discovered by a user on this forum thread: https://discuss.openedx.org/t/xblock-django-installation-failiure/1802
There was a problem hiding this comment.
@swalladge, not really, no. This PR was to update requirements so they would work for Django 2.2 and Python 3. There was a previous PR (https://github.com/edx/xblock-sdk/pull/165/files) for Django 2.2 support, but requirements wouldn't compile successfully still, which is why I opened this PR. I see that README.md still says Python 2.7 though, which is misleading now.
Unfortunately pip-compile completely ignores python_version directives, and they've stated they won't support them, so there isn't really a good way to have different django requirements for different python versions as long as we're using pip-tools.
Should we add extra documentation to note which versions of xblock-utils are Django 1.11 compatible vs Django 2.2?
There was a problem hiding this comment.
Should we add extra documentation to note which versions of xblock-utils are Django 1.11 compatible vs Django 2.2?
Probably yes. Currently, python2 support has been unexpectedly dropped with this PR. It sounds like formally dropping python2 is going to happen soon anyway; it's just happened sooner than expected. :P
openedx-webhooks
added
merged
and removed
waiting on author
When updating requirements for open-craft/problem-builder#262, we were unable to get pip-compile to resolve dependencies, as the xblock inherits requirements from xblock-sdk, and there were unresolvable dependencies.
This updates 2 requirements to allow pip-upgrade to work in both xblock-sdk and problem-builder requirements.
Reviewers