Document Requests kwargs #679
Conversation
Deploying logfire-docs with
|
| Latest commit: |
6b59d52
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://20c383c5.logfire-docs.pages.dev |
| Branch Preview URL: | https://document-kwargs-celery.logfire-docs.pages.dev |
logfire/__init__.py
Outdated
| 'StructlogProcessor', | ||
| 'LogfireLoggingHandler', | ||
| 'loguru_handler', | ||
| 'SamplingOptions', | ||
| 'MetricsOptions', | ||
| 'CeleryInstrumentKwargs', |
There was a problem hiding this comment.
I think we should in general avoid adding things to __init__ if they only serve as type hints.
Kludex
force-pushed
the
document-kwargs-celery
branch
from
afb3724 to
c4a4f03
Compare
|
@alexmojaki please confirm if this is okay for you. If it's, I'll add the others. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #679 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 137 137
Lines 10919 10920 +1
Branches 1520 1520
=========================================
+ Hits 10919 10920 +1 ☔ View full report in Codecov by Sentry. |
| @@ -1174,17 +1174,26 @@ def instrument_httpx( | |||
| self._warn_if_not_initialized_for_instrumentation() | |||
| return instrument_httpx(self, client, **kwargs) | |||
|
|
|||
| def instrument_celery(self, **kwargs: Unpack[CeleryInstrumentKwargs]) -> None: | |||
| def instrument_celery(self, **kwargs: Any) -> None: | |||
There was a problem hiding this comment.
What about skip_dep_check?
There was a problem hiding this comment.
I removed on purpose. Is it really important? People shouldn't use it anyway... Unless specific cases like psycopg-binary...
There was a problem hiding this comment.
No it's not that important but I wasn't sure if it was intentional. Let's leave it out.
| def instrument_requests( | ||
| self, | ||
| excluded_urls: str | None = None, | ||
| request_hook: Callable[[Span, requests.PreparedRequest], None] | None = None, |
There was a problem hiding this comment.
Nice of otel to pass the actual request object this time lol
|
let me know if this is okay, and I'll apply to the others. |
Kludex
force-pushed
the
document-kwargs-celery
branch
from
c4bc011 to
e4ce95e
Compare
Kludex
force-pushed
the
document-kwargs-celery
branch
from
e4ce95e to
6b59d52
Compare
Should we actually remove the kwargs and use the parameters? It's just a bad experience in every sense to have kwargs.