-
Notifications
You must be signed in to change notification settings - Fork 27
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
🎨 introduce logger filtering with ENV var ⚠️ #6596
🎨 introduce logger filtering with ENV var ⚠️ #6596
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6596 +/- ##
========================================
Coverage 87.88% 87.89%
========================================
Files 1562 1556 -6
Lines 62829 62680 -149
Branches 2085 2088 +3
========================================
- Hits 55218 55091 -127
+ Misses 7293 7271 -22
Partials 318 318
Continue to review full report in Codecov by Sentry.
|
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.
added a comment
…atusdrobuliak66/osparc-simcore into maintenance/reduce-uvicorn-access-logs
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.
I have a subtle change request that I personally find reasonable, but I have formulated as a comment. If you dont want to do it, please bring me a coffee or tell me a compliment, then I might overlook my precautions and approve 😽
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, just a few comments
@matusdrobuliak66 this is amazing. I like it! I wonder if it further slows down general performance because of logging. It would be nice to see a benchmark but we don't have anything set up yet to compare. |
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.
I think before it is merged, we need a PR in osparc-config
with all the new required ENV.
For the record re regex speed: you are right @matusdrobuliak66 https://stackoverflow.com/questions/19911508/python-speed-for-in-vs-regular-expression |
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.
I like the idea. I have some comments regarding the implementation. Lots of thoughts :-)
I am happy to discuss it offline and even help implementing some parts (now or in further iterations)
services/web/server/src/simcore_service_webserver/application_settings.py
Show resolved
Hide resolved
services/autoscaling/src/simcore_service_autoscaling/core/settings.py
Outdated
Show resolved
Hide resolved
services/autoscaling/src/simcore_service_autoscaling/core/settings.py
Outdated
Show resolved
Hide resolved
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.
Please consider the comments before merging. Either by taking action in this PR or by creating an issue.
services/api-server/src/simcore_service_api_server/core/settings.py
Outdated
Show resolved
Hide resolved
services/clusters-keeper/src/simcore_service_clusters_keeper/core/settings.py
Outdated
Show resolved
Hide resolved
services/dask-sidecar/src/simcore_service_dask_sidecar/settings.py
Outdated
Show resolved
Hide resolved
services/payments/src/simcore_service_payments/core/settings.py
Outdated
Show resolved
Hide resolved
services/resource-usage-tracker/src/simcore_service_resource_usage_tracker/core/settings.py
Outdated
Show resolved
Hide resolved
services/web/server/src/simcore_service_webserver/application_settings.py
Outdated
Show resolved
Hide resolved
services/autoscaling/src/simcore_service_autoscaling/core/settings.py
Outdated
Show resolved
Hide resolved
|
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. You missed one.
services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/settings.py
Show resolved
Hide resolved
357a841
into
ITISFoundation:master
What do these changes do?
LOG_FILTER_MAPPING
Example:
Related issue/s
How to test
Dev-ops checklist