feat(tracing) Change the return type of the native execute #2279
Conversation
It is very difficult to capture new data from Clickhouse with the current return value from the function. The current wrapper around the clickhouse-driver library returns a simple list of results, that gets overloaded with column types when that meta information is returned. Change the return type to be an object, so that any data that we might want to propogate up from Clickhouse becomes easy to add without having to go through this process again.
snuba/clickhouse/native.py
Outdated
| new_result = {"data": data, "meta": meta} | ||
| new_result = {"data": data, "meta": meta, "profile": profile} |
There was a problem hiding this comment.
Does that mean our API is returning "profile" information now?
There was a problem hiding this comment.
Yes, the data is there, just not being used yet. I am going to make use of it in separate PR(s).
snuba/clickhouse/native.py
Outdated
| class ClickhouseResult: | ||
| results: Sequence[Any] = field(default_factory=list) | ||
| meta: Sequence[Any] | None = None | ||
| profile: Mapping[str, Any] | None = None |
There was a problem hiding this comment.
Can we provide something more specific ?
Like having a datclass for the profile information. I am not sure what we gain from leaving it as a free form mapping knowing that the profile information is static and well known.
There was a problem hiding this comment.
I removed the profiling data from this PR, it's not really this is about. I'll add it back in a separate PR.
Codecov Report
@@ Coverage Diff @@
## master #2279 +/- ##
=======================================
Coverage 93.06% 93.06%
=======================================
Files 553 553
Lines 25238 25255 +17
=======================================
+ Hits 23487 23503 +16
- Misses 1751 1752 +1
Continue to review full report at Codecov.
|
It is very difficult to capture new data from Clickhouse with the current return
value from the function. The current wrapper around the clickhouse-driver
library returns a simple list of results, that gets overloaded with column types
when that meta information is returned. Change the return type to be an object,
so that any data that we might want to propagate up from Clickhouse becomes easy
to add without having to go through this process again.