Skip to content

Add CDM Timeseries Upload Queue#439

Merged
devendra-lohar merged 28 commits into
masterfrom
DOG-5255-CDM-Timeseries-db-extractor
Jun 25, 2025
Merged

Add CDM Timeseries Upload Queue#439
devendra-lohar merged 28 commits into
masterfrom
DOG-5255-CDM-Timeseries-db-extractor

Conversation

@devendra-lohar

@devendra-lohar devendra-lohar commented May 8, 2025

Copy link
Copy Markdown
Contributor

Add CDM Timeseries Upload Queue support for Timeseries Destination in DB Extractor.

@codecov

codecov Bot commented May 9, 2025

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 86.32479% with 16 lines in your changes missing coverage. Please review.

Project coverage is 77.99%. Comparing base (614939f) to head (f024e28).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
cognite/extractorutils/uploader/time_series.py 86.53% 14 Missing ⚠️
cognite/extractorutils/uploader_extractor.py 75.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #439      +/-   ##
==========================================
+ Coverage   77.71%   77.99%   +0.27%     
==========================================
  Files          42       42              
  Lines        3523     3613      +90     
==========================================
+ Hits         2738     2818      +80     
- Misses        785      795      +10     
Files with missing lines Coverage Δ
cognite/extractorutils/uploader/__init__.py 100.00% <ø> (ø)
cognite/extractorutils/uploader_types.py 95.83% <100.00%> (+1.09%) ⬆️
cognite/extractorutils/uploader_extractor.py 79.16% <75.00%> (-1.14%) ⬇️
cognite/extractorutils/uploader/time_series.py 76.54% <86.53%> (+4.29%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@devendra-lohar
devendra-lohar marked this pull request as ready for review May 14, 2025 05:24
@devendra-lohar
devendra-lohar requested a review from a team as a code owner May 14, 2025 05:24
Comment thread cognite/extractorutils/uploader/time_series.py Outdated
Comment thread cognite/extractorutils/uploader/time_series.py
Comment thread cognite/extractorutils/util.py Outdated
@devendra-lohar
devendra-lohar requested a review from toondaey May 19, 2025 08:20
Comment thread cognite/extractorutils/util.py Outdated
Comment thread cognite/extractorutils/uploader/time_series.py Outdated
@devendra-lohar
devendra-lohar requested a review from mathialo May 22, 2025 08:32
Comment thread cognite/extractorutils/uploader/time_series.py Outdated
Comment thread cognite/extractorutils/util.py Outdated
Comment thread cognite/extractorutils/uploader/time_series.py Outdated

@mathialo mathialo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generic approach is pretty nice!

Noe more nitpick, but this is shaping up quite well

Comment thread cognite/extractorutils/uploader/time_series.py Outdated
@devendra-lohar
devendra-lohar requested a review from toondaey June 4, 2025 13:58
@devendra-lohar
devendra-lohar requested a review from mathialo June 11, 2025 10:33
mathialo
mathialo previously approved these changes Jun 16, 2025

@mathialo mathialo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@mathialo mathialo added the waiting-for-risk-review Waiting for a member of the risk review team to take an action label Jun 16, 2025
@thorkildcognite thorkildcognite added the risk-review-exemption-big-pr Approved exemption for large PR (>500 lines) label Jun 20, 2025
@rsjr rsjr self-assigned this Jun 20, 2025
@rsjr rsjr added risk-review-ongoing Risk review is in progress and removed waiting-for-risk-review Waiting for a member of the risk review team to take an action labels Jun 20, 2025

@rsjr rsjr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. However, I suggest adding two more integration test cases:

  • A test case ingesting datapoints with status codes (similar to this)
  • A non-happy path test case just to cover the behavior when ingesting datapoints with wrong type to an existing time series: the time series is numeric but you try to ingest string datapoints (or vice-versa).

@rsjr rsjr added the waiting-for-team Waiting for the submitter or reviewer of the PR to take an action label Jun 20, 2025
@devendra-lohar
devendra-lohar requested a review from rsjr June 24, 2025 10:15
@devendra-lohar devendra-lohar added the waiting-for-risk-review Waiting for a member of the risk review team to take an action label Jun 24, 2025

@rsjr rsjr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix you added here 8e63e36

This doesn't look to be related to the changes in this PR? Is it fixing some other side effect from somewhere else?

@rsjr rsjr removed the waiting-for-risk-review Waiting for a member of the risk review team to take an action label Jun 24, 2025
@devendra-lohar

Copy link
Copy Markdown
Contributor Author

This fix you added here 8e63e36

This doesn't look to be related to the changes in this PR? Is it fixing some other side effect from somewhere else?

@rsjr
Yes this does not directly relate to the current PR but rather is inline with the latest release on cognite-sdk and fixing the resulting mypy error being caused.

@devendra-lohar
devendra-lohar requested a review from rsjr June 24, 2025 15:27
@devendra-lohar devendra-lohar added the waiting-for-risk-review Waiting for a member of the risk review team to take an action label Jun 25, 2025

@rsjr rsjr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

risk review ok

@devendra-lohar
devendra-lohar merged commit 6275423 into master Jun 25, 2025
6 checks passed
@devendra-lohar
devendra-lohar deleted the DOG-5255-CDM-Timeseries-db-extractor branch June 25, 2025 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk-review-exemption-big-pr Approved exemption for large PR (>500 lines) risk-review-ongoing Risk review is in progress waiting-for-risk-review Waiting for a member of the risk review team to take an action waiting-for-team Waiting for the submitter or reviewer of the PR to take an action

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants