Skip to content
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

Makes statsd optional #47

Open
ChrisLovering opened this issue Apr 2, 2022 · 1 comment
Open

Makes statsd optional #47

ChrisLovering opened this issue Apr 2, 2022 · 1 comment
Labels
a: code Pull requests which add features, fixes, or any code change s: planning A feature being considered or discussed t: enhancement

Comments

@ChrisLovering
Copy link
Member

In #42 we tried to make statsd optional while adding it, however the way we attempted to do so made it so that editors wouldn't detect that the async statsd client inherited methods from stats client base.

@HassanAbouelela HassanAbouelela added s: planning A feature being considered or discussed a: code Pull requests which add features, fixes, or any code change t: enhancement labels Jul 23, 2022
@wookie184
Copy link
Contributor

If we want to make statsd an optional dependency we shouldn't expose StatsClientBase directly (or by inheriting from it). Instead we should create a wrapper class (e.g. an ABC), with one implementation that uses statsd, and have a separate implementation (that doesn't do anything) for when statsd is not installed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: code Pull requests which add features, fixes, or any code change s: planning A feature being considered or discussed t: enhancement
Projects
None yet
Development

No branches or pull requests

3 participants