-
Notifications
You must be signed in to change notification settings - Fork 14.9k
docs: pypi-installation on Ubuntu 24.04 and statsd package for event-logging #32891
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
base: master
Are you sure you want to change the base?
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.
I've completed my review and didn't find any issues.
Need a new review? Comment
/korbit-review
on this PR and I'll review your latest changes.Korbit Guide: Usage and Customization
Interacting with Korbit
- You can manually ask Korbit to review your PR using the
/korbit-review
command in a comment at the root of your PR.- You can ask Korbit to generate a new PR description using the
/korbit-generate-pr-description
command in any comment on your PR.- Too many Korbit comments? I can resolve all my comment threads if you use the
/korbit-resolve
command in any comment on your PR.- On any given comment that Korbit raises on your pull request, you can have a discussion with Korbit by replying to the comment.
- Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.
Customizing Korbit
- Check out our docs on how you can make Korbit work best for you and your team.
- Customize Korbit for your organization through the Korbit Console.
Current Korbit Configuration
General Settings
Setting Value Review Schedule Automatic excluding drafts Max Issue Count 10 Automatic PR Descriptions ❌ Issue Categories
Category Enabled Documentation ✅ Logging ✅ Error Handling ✅ Readability ✅ Design ✅ Performance ✅ Security ✅ Functionality ✅ Feedback and Support
Note
Korbit Pro is free for open source projects 🎉
Looking to add Korbit to your team? Get started with a free 2 week trial here
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.
|
||
```python | ||
from superset.stats_logger import StatsdStatsLogger | ||
STATS_LOGGER = StatsdStatsLogger(host='localhost', port=8125, prefix='superset') | ||
``` | ||
|
||
If you get the Error `ImportError: cannot import name 'StatsdStatsLogger' from 'superset.stats_logger' (/path/to/superset/env/superset/stats_logger.py)`, you can edit the `except Exception` block to print error details: |
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.
What file is being edited here? It's unclear to me, sorry.
@@ -51,11 +51,24 @@ if desired. Most endpoints hit are logged as | |||
well as key events like query start and end in SQL Lab. | |||
|
|||
To setup StatsD logging, it’s a matter of configuring the logger in your `superset_config.py`. | |||
If not already present, you need to ensure that the `statsd`-package is installed in superset's python environment. |
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.
Nitpick: could you capitalize Superset throughout?
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 two tiny things (capitalize Superset and clarify what file to edit) but this LGTM. @125m125 do you want to make those tweaks? That would be ideal but if it gets stale I'll just merge it.
@125m125 this is fine with me. Would you like to do that on a separate PR after this merges, or add it to this PR (in which case would you take out the docs about modifying the logger function) ? |
@sfirke Thank you for the feedback, I have capitalized Superset and implemented the error-handling suggestion. I have done it in two commits in case you prefer the error handling to be a separate PR, since it isn't only documentation anymore. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #32891 +/- ##
===========================================
+ Coverage 60.48% 83.38% +22.89%
===========================================
Files 1931 550 -1381
Lines 76236 39510 -36726
Branches 8568 0 -8568
===========================================
- Hits 46114 32946 -13168
+ Misses 28017 6564 -21453
+ Partials 2105 0 -2105
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
SUMMARY
StatsdStatsLogger
class but it throws the caught exception on init so the users can see the actual problem?BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION