Log detailed elasticsearch params #139
Merged
Conversation
This file contains 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
the search_string helper method just obscures the importance of the args
ci uses tox, which uses the manage.py test that requires a running elastic
ashultz0
force-pushed
the
ashultz0/detailed-elastic-params
branch
from
6ddc767 to
c4d4c6e
Compare
|
codecov is complaining because of one line not covered (the actual log), doesn't seem worth a whole test case. |
schenedx
approved these changes
Nov 15, 2023
We don't want to trigger logging on every single ES search or force logging into other courseware search uses, so arg + setting just for courseware search. Fairly awkward test brought to you via the repository's 100% coverage policy, but it does ensure the log line is reached and does not error.
ashultz0
force-pushed
the
ashultz0/detailed-elastic-params
branch
from
c4d4c6e to
7b4df58
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
We need to know exactly what we are asking elasticsearch sometimes to debug problems. But we would prefer not to log it for all uses or force logging on openedx in general.
After a bunch of cleanup to make working in here a little easier and a new make target for local installation, I added a new log parameter to the general search arg and activates it for elasticsearch. Then I added a setting to turn this flag on for courseware search which is the place we currently need it, other settings can come later if desired.
After this is ready I'll add a default setting of False in platform lms and a devstack setting of True, and then some settings for edX proper in the internal settings repo to turn it on.
Sample local log