-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
build: Pin social-auth-app-django as a requirement. #35237
Conversation
7e2b035
to
b16a4f2
Compare
5abc3f2
to
1dc95ee
Compare
requirements/edx/base.txt
Outdated
firebase-admin==6.5.0 | ||
# via edx-ace |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not your fault @dianakhuang , but I am now realizing that the new version of edx-ace is pulling in an absolute pile of new google dependencies 😓
I am going to run this by my team and see whether or not those new deps are an expected outcome of the recent mobile notifications project. I should know by 2pm whether we're going to let these transitive deps in, or whether we're going to pin+fix edx-ace.
Up to you whether you want to make this PR just pin social-auth-app-django vs. wait for me while I figure out this edx-ace thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I will roll back the general upgrade and focus on the pin for now. Good luck with figuring out edx-ace. 🫡
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM @dianakhuang and thanks for the quick turnaround on the pin. In that case my only other request is that you make this a build:
rather than a feat:
.
There are a few migrations going into this library which cause operational headaches for operators. We would like to pin until the migrations settle down and then we can unpin this again.
1dc95ee
to
7199e61
Compare
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
2U Release Notice: This PR has been deployed to the edX production environment. |
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
2U Release Notice: This PR has been deployed to the edX production environment. |
2U Release Notice: This PR has been rolled back from the edX production environment. |
There are a few migrations going into this library which cause operational headaches for operators.
We would like to pin until the migrations settle down and then we can unpin this again.