-
Notifications
You must be signed in to change notification settings - Fork 455
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
Fix: Remove base_url from grades_download #999
Fix: Remove base_url from grades_download #999
Conversation
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.
Thanks for the PR! Did you check if CSV profile information download still works without the minio plugin after the fix?
@@ -0,0 +1 @@ | |||
[Improvement] Remove base_url from GRADES_DOWNLOAD. (by @FahadKhalid210) |
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.
This is actually a bugfix; and we should not describe what we are doing in this commit, but why. The "what" is removing the base_url
entry. The "why" is to prevent a 500 error caused by an ImproperlyConfigured exception when downloading profile information as CSV, and when the minio plugin is enabled.
So I would rewrite this changelog entry as follows:
- [Bugfix] When the minio plugin is enabled (particularly on Kubernetes), fix a 500 error on downloading profile information as CSV. (by @FahadKhalid210)
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.
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.
OK so it would seem that we should not be pushing this fix in Tutor core, but in the minio plugin, right?
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.
Since this fix pertains to the minio plugin, it would be more suitable to address it there. However, let me investigate if we can fix this url issue(404) in Tutor core.
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.
@regisb URL issue is fixed by updating the location
.
I have tested this fix with and without minio plugin and it works fine now.
URL without minio plugin:
http://local.overhang.io:8000/media/bb6d638a296059742509a0d319ddf8456b6dbf9a/edX_DemoX_Demo_Course_student_profile_info_2024-02-09-1101.csv
URL with minio plugin:
http://files.local.overhang.io:9000/openedx/openedx/media/bb6d638a296059742509a0d319ddf8456b6dbf9a/edX_DemoX_Demo_Course_student_profile_info_2024-02-09-1104.csv
Closing this as PR is created in minio plugin |
Getting Error when click on
Download profile information as a CSV
.Steps to reproduce:
Launch Tutor with
Quince
version and withminio
plugin.In the LMS, head to Data Downloads section under the Instrutor tab. Click the
Download profile information as a CSV
.The download will fail and LMS logs will show below mentioned error
See the initial conversation here: hastexo/tutor-contrib-s3#70 (comment)