Bump edx-organizations version to 0.4.0 - #11559
Conversation
| django==1.8.9 | ||
| djangorestframework-jwt==1.7.2 | ||
| edx-django-oauth2-provider==0.5.0 | ||
| edx-djangorestframework-oauth==1.0.1 |
There was a problem hiding this comment.
Can we find a way to get rid of this fork? We specify edx-django-oauth2-provider immediately above. If we were to use djangorestframework-oauth and move edx-django-oauth2-provider afterward, would this resolve the issue? Is there a way to have pip override the dependency?
There was a problem hiding this comment.
I would also like to get rid of this fork. Shadowing django-oauth2-provider by installing edx-django-oauth2-provider after djangorestframework-oauth would resolve the issue, but it seems brittle to me. Would a comment about the install order suffice? I haven't found a clean, explicit way to have pip override the dependency.
Changes to the edx-djangorestframework-oauth fork were introduced in edx/django-rest-framework-oauth#1 during the DRF upgrade. @macdiesel any chance you have something to add here?
There was a problem hiding this comment.
Adding a comment, and adjusting the order seems a better approach than having our own fork.
There was a problem hiding this comment.
The only thing the edx fork of djangorestframework-oauth is doing is modifying the requirements and setup.py. The project has an open PR to remove the django-oauth2-provider requirements from the runtime environment: https://github.com/jpadilla/django-rest-framework-oauth/pull/8/files.
I would be cool with dumping this fork if we could get that PR pushed upstream to remove that dependency. I agree with you @rlucioni having to order these dependencies and shadow install the correct versions is brittle and I would prefer we do not adopt this habit.
There was a problem hiding this comment.
Thanks @macdiesel. I'll see what I can do to get the dependencies removed from the upstream project.
There was a problem hiding this comment.
I've opened jpadilla/django-rest-framework-oauth#12.
There was a problem hiding this comment.
jpadilla/django-rest-framework-oauth#12 was merged! I'll update this PR so that we use the upstream version of the package.
215fdcc to
9fa7fb5
Compare
|
👍 |
9fa7fb5 to
2423ad4
Compare
|
👍 |
|
@nedbat and @maxrothman, this PR uninstalls a package formerly installed as editable. Based on what we learned in https://github.com/edx/edx-platform/pull/11432, it sounds like this will require building an AMI from scratch. I'm tagging the two of you now to make you aware of this before the 2/29 release you'll be managing together. Please let me know if you have any questions. |
|
@rlucioni why'd you ping me specifically? I think anyone in @edx/devops should be able to review. |
|
@maxrothman because you're listed as being on-call for the 2/29 release and I didn't want to blast everyone with an email. |
|
So you plan on deploying this the week after next? |
|
@maxrothman yes; next week's release has already been cut. |
|
Great, thanks for the heads up. Let's communicate more about this towards the end of next week. |
2423ad4 to
b4e882b
Compare
Also install djangorestframework-oauth 1.1.0 from PyPI
b4e882b to
79a4d6c
Compare
Bump edx-organizations version to 0.4.0
5 participants
Also install djangorestframework-oauth from PyPI. Requires openedx/edx-organizations#25.
@jimabramson or @clintonb please review.