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

filecontent: report itemsRead metric #1002

Merged
merged 3 commits into from
Dec 18, 2024
Merged

filecontent: report itemsRead metric #1002

merged 3 commits into from
Dec 18, 2024

Conversation

dmivankov
Copy link
Contributor

Since filecontent doesn't go through SdkV1Relation.{buildScan, getStream} to implement reads
read metrics weren't being automatically reported.

Now it will do itemsRead.inc() once per line

Since filecontent doesn't go through `SdkV1Relation.{buildScan, getStream}` to implement reads
read metrics weren't being automatically reported.

Now it will do `itemsRead.inc()` once per line
@Jacob-Eliat-Eliat
Copy link
Contributor

Do you remember why we decided not to implement sdkV1relation? I vaguely remember discussing it but I don't remember the reasons for our decisions back then

@dmivankov
Copy link
Contributor Author

Do you remember why we decided not to implement sdkV1relation? I vaguely remember discussing it but I don't remember the reasons for our decisions back then

Nope, don't remember any specifics, maybe it doesn't provide suitable helpers or assumes more operations that filecontent supports? But could be something to look into

Copy link

codecov bot commented Dec 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.33%. Comparing base (410e714) to head (9a6cec0).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1002   +/-   ##
=======================================
  Coverage   83.33%   83.33%           
=======================================
  Files          47       47           
  Lines        3156     3156           
  Branches      452      452           
=======================================
  Hits         2630     2630           
  Misses        526      526           
Files with missing lines Coverage Δ
...n/scala/cognite/spark/v1/FileContentRelation.scala 95.23% <ø> (ø)

@dmivankov dmivankov enabled auto-merge (squash) December 18, 2024 12:59
@dmivankov dmivankov merged commit 3902217 into master Dec 18, 2024
4 checks passed
@dmivankov dmivankov deleted the filecontent_metric branch December 18, 2024 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants