SE-2170 Port changes not yet merged to upstream into our juniper release branch - #225
Merged
pomegranited merged 6 commits intoJul 27, 2020
Merged
Conversation
cf https://github.com/edx/edx-platform/pull/20957 Squashed commit of the following: commit 127a521 Author: pkulkark <pooja@opencraft.com> Date: Tue Apr 21 13:25:59 2020 +0530 refactor code based on latest master commit e6ccac4 Merge: fead182 f40bb34 Author: pkulkark <pooja@opencraft.com> Date: Tue Apr 21 13:04:41 2020 +0530 Merge branch 'master' of github.com:edx/edx-platform into pooja/optionally-disable-registration-fields commit fead182 Merge: 00ada9e c78db13 Author: pkulkark <pooja@opencraft.com> Date: Thu Aug 1 13:29:55 2019 +0530 Merge branch 'master' of github.com:edx/edx-platform into pooja/optionally-disable-registration-fields commit 00ada9e Merge: 1409985 3e2e815 Author: pkulkark <pooja@opencraft.com> Date: Thu Aug 1 12:16:22 2019 +0530 Merge branch 'master' of github.com:edx/edx-platform into pooja/optionally-disable-registration-fields commit 1409985 Author: pkulkark <pooja@opencraft.com> Date: Thu Aug 1 11:22:37 2019 +0530 Fix quality commit dc04d47 Merge: 2122988 b5a8f67 Author: pkulkark <pooja@opencraft.com> Date: Thu Aug 1 10:49:25 2019 +0530 Merge branch 'master' of github.com:edx/edx-platform into pooja/optionally-disable-registration-fields commit 2122988 Merge: 4495c3b 9076b5d Author: pkulkark <pooja@opencraft.com> Date: Wed Jul 31 18:29:24 2019 +0530 Merge branch 'master' of github.com:edx/edx-platform into pooja/optionally-disable-registration-fields commit 4495c3b Author: pkulkark <pooja@opencraft.com> Date: Tue Jul 30 16:22:23 2019 +0530 Modify check condition commit 05e0fad Merge: fa0c169 cf74aee Author: pkulkark <pooja@opencraft.com> Date: Tue Jul 30 14:58:15 2019 +0530 Merge branch 'master' of github.com:edx/edx-platform into pooja/optionally-disable-registration-fields commit fa0c169 Author: pkulkark <pooja@opencraft.com> Date: Thu Jul 18 10:50:25 2019 +0530 Fix quality issue commit 92cd44d Author: pkulkark <pooja@opencraft.com> Date: Sun Jul 14 12:35:28 2019 +0530 Remove read-only override from login form commit c53bddb Merge: 9e7cf61 1e6d6de Author: pkulkark <pooja@opencraft.com> Date: Fri Jul 12 11:13:11 2019 +0530 Merge branch 'master' of github.com:edx/edx-platform into pooja/optionally-disable-registration-fields commit 9e7cf61 Author: pkulkark <pooja@opencraft.com> Date: Fri Jul 12 11:06:37 2019 +0530 Add check for other settings + unittest commit ffdb607 Author: pkulkark <pooja@opencraft.com> Date: Tue Jul 9 12:55:55 2019 +0530 Add override in required method that was missed commit 8457746 Author: pkulkark <pooja@opencraft.com> Date: Mon Jul 8 14:46:45 2019 +0530 Optionally disable registration fields This fix adds the ability to optionally disable fields in the registration form when using third party authentication.
…tory endpoint cf https://github.com/edx/edx-platform/pull/20948 Squashed commit of the following: commit edc116f Author: Josue Balandrano Coronel <jbc@rmcomplexity.com> Date: Tue Jul 16 13:52:25 2019 -0500 Name submission history view. commit abbe321 Author: Josue Balandrano Coronel <jbc@rmcomplexity.com> Date: Mon Jul 15 08:52:45 2019 -0500 Improve documentation for submission history. commit 6d3afeb Author: Josue Balandrano Coronel <jbc@rmcomplexity.com> Date: Wed Jul 3 17:26:36 2019 -0500 Update enrollment serializer to add finished courses and grades. Add problem submission history endpoint.
pomegranited
marked this pull request as draft
July 17, 2020 09:30
pomegranited
force-pushed
the
jill/juniper-code-drift
branch
from
July 18, 2020 01:52
0583f47 to
272881b
Compare
1 task
cf https://github.com/edx/edx-platform/pull/23731 Move Celery check task to the high priority queue Currently, LMS uses 3 Celery workers: lms_default_1, lms_high_1 and lms_high_mem_1. Each Celery worker sends messages to a single queue: edx.core.default, edx.core.high and edx.core.high_mem, respectively. The number of child processes per Celery worker is set to 1. Due to this configuration, any task in a queue blocks all other tasks. Currently, the Celery check task submitted by the /heartbeat?extended LMS HTTP API endpoint runs in the default queue. When some slow task (eg course grades creation) is sent to the default queue, it will block the Celery check task, which will expire and the heartbeat endpoint will fail. This patch moves the task to another queue which has only shorter tasks and in which this problem will not occur. Use a Django setting for the Celery check task routing key so that it can be overriden by individual OpenEDX instances via JSON env files. (cherry picked from commit eaf229556365889d66ac7c9baeb866df0d659e11)
cf https://github.com/edx/edx-platform/pull/23735 Unhide student-generated certificates toggle Student-generated certificates are meant to be auto-enabled through signal for self-paced courses, but it has been observed that this behaviour is flaky. When this happens, we are left in a state where it is impossible to enable student-generated certificates because the section is hidden in the instructor dashboard. Also by unconditionally displaying the toggle button, this allows more flexibility for instructors. For example, it may be desirable for student-generated certificates to be disabled for a self-paced course. (cherry picked from commit 86acd244d964e6281f3564d14e370e5bc79c2fbb)
…Block cf https://github.com/edx/edx-platform/pull/24343 Fix the order randomization behaviour of Randomized Content Block The Randomized Content Block XBlock only randomizes the selection of the children blocks and has unpredictable randomization of the order of the selected child blocks due to the usage of sets, which are unordered, for storing the selected blocks. This becomes apparent when all the available child blocks in a library are chosen for a Randomized Content Block, to randomize just the order of the child blocks and not just the selection of the blocks. The order of the selected blocks ends up being similar for multiple learners. This change modifies the XBlock to store the selected child blocks in a list, instead of a set, after randomly shuffling them. It also adds a new block transformer 'ContentLibraryTransformer', which randomizes the order of the selected blocks. (cherry picked from commit 967688f86718ebd7fa9b53ac05a482e4b4dc5782) Address review comment (cherry picked from commit 2fab6dc8954f214a279d3ab6752f93470ffd2ebf)
…ble-changing-require-build-profile BB-1156: Enable changing the build profile used by RequireJS (cherry picked from commit ee61c9d)
pomegranited
force-pushed
the
jill/juniper-code-drift
branch
from
July 18, 2020 07:32
16b8364 to
eb4a14c
Compare
pomegranited
marked this pull request as ready for review
July 18, 2020 08:05
nizarmah
self-requested a review
July 18, 2020 11:29
|
I'm done with the "Overall Tests". But I had a particular issue with the following step:
I am getting a Server Error when trying to do that for a particular course. I checked, however, the instance settings and I couldn't see anything analytics related. Therefore, I'll assume that you aren't using analytics, which is causing that error. Correct me if I'm wrong please. Status of reviewing the cherry picked commits:
I wasn't able to test the 4f75b11 commit. The testing instructions on edx#23731 were not clear, so I had no idea how to test it But I think everything should be working correctly. |
Author
Yep, we're not using analytics, so the URL it goes to doesn't resolve to anything. All good there!
To test this, I ran this on the appserver: sudo tail -f /edx/var/log/**/*.logand watched the logs while I hit https://pr225.sandbox.opencraft.hosting/heartbeat?extended The name of the worker log file should show that the celery task uses the high priority queue. Does that test it sufficiently @kaizoku ? |
|
Alright, it took me so much time, but I finally did it. Even though that endpoint resulted in a server error, the logs were in Everything here should be tested already, what is left is open-craft/configuration#129 |
nizarmah
approved these changes
Jul 20, 2020
There was a problem hiding this comment.
👍
- I tested this through:
- Overall testing
- Testing each cherry picked pull request:
- Read through the code
- Followed the testing instructions for each pull request
- I read through the code
-
I checked for accessibility issues -
Includes documentation -
I made sure any change in configuration variables is reflected in the corresponding client'sconfiguration-securerepository.
|
@pomegranited yes, that's a sufficient test. The only difference we'll see here is that it runs in the high priority queue, so either checking the active tasks (hard to catch since this task runs quickly), or the logfiles is our best test here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This branch starts with the tagged upstream
open-release/juniper.2, and adds in the code drift that was present inopencraft-release/ironwood.2which has not yet been merged upstream.For PRs where there was only a single commit, I did a simple cherry-pick.
But where there were many commits on the PR, I didn't want to clutter up our repo. So I squashed and rebased on juniper, and then cherry-picked that commit here.
Testing instructions
Sandbox is provisioning:
Overall:
For each commit:
Reviewer
CC @lgp171188