-
Notifications
You must be signed in to change notification settings - Fork 389
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
chore: set logger level to error to reduce noise from Elasticsearch and OpenSearch client libraries #4979
chore: set logger level to error to reduce noise from Elasticsearch and OpenSearch client libraries #4979
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/v2.0.0 #4979 +/- ##
================================================
+ Coverage 60.92% 91.18% +30.26%
================================================
Files 329 136 -193
Lines 17674 5855 -11819
================================================
- Hits 10767 5339 -5428
+ Misses 6907 516 -6391
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
search_engine: Union[ | ||
Literal[SEARCH_ENGINE_ELASTICSEARCH], | ||
Literal[SEARCH_ENGINE_OPENSEARCH], | ||
] = SEARCH_ENGINE_ELASTICSEARCH |
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.
Even if we can define constants for official engines, we shouldn't fix the available options.
Creating new custom engine or specific engine connection could be helpful for the community but, by fixing this we’re avoiding this possibility. (See how engine are registered)
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.
But in the case that a PR from the community adds a new engine they should modify this Union
as part of the PR right? I don't get the problem.
(See how engine are registered)
I missed that one. I will use the constant there too.
return self.search_engine == SEARCH_ENGINE_OPENSEARCH | ||
|
||
@property | ||
def humanized_search_engine(self) -> str: |
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.
The name of this method gives me chills...
"I've seen things you people wouldn't believe. Attack ships on fire off the shoulder of Orion. I watched C-beams glitter in the dark near the Tannhäuser Gate. All those moments will be lost in time like tears in rain..."
Could we just call human_readable_search_engine
? And also generalize a bit by using str.title() ? I think we don't need al this extra functions if we keep simple
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.
"Humanize" is a very common concept and naming when formatting strings but I don't have any problem changing it to "human_readable_*". 🙂
Some examples:
- One Python library: https://python-humanize.readthedocs.io/en/latest/
- A Django module: https://docs.djangoproject.com/en/5.0/ref/contrib/humanize/
- Moment.js classic JS library: https://momentjs.com/docs/#/durations/humanize/
- Size: an open source library that I made some years ago https://github.com/jfcalvo/size?tab=readme-ov-file#humanize-file-sizes
@frascuchon I applied feedback from our discussion:
|
Description
This PR includes some changes to reduce the logger level of Elasticsearch and OpenSearch to error. With this we are reducing a little bit the output noise of these client libraries.
I have improved a little bit the settings so only
elasticsearch
andopensearch
are valid options forsearch_engine
attribute. I have added some small useful functions too.Refs #4946
Type of change
(Please delete options that are not relevant. Remember to title the PR according to the type of change)
How Has This Been Tested
(Please describe the tests that you ran to verify your changes. And ideally, reference
tests
)Checklist