Skip to content

Set start time of extractors.#458

Merged
toondaey merged 3 commits intomasterfrom
set-start-time/dog-5786
Jul 17, 2025
Merged

Set start time of extractors.#458
toondaey merged 3 commits intomasterfrom
set-start-time/dog-5786

Conversation

@toondaey
Copy link
Copy Markdown
Contributor

This simply does nothing right now but just set the start time of the extractor. This will be used for one, when we report the startup.

@toondaey toondaey requested a review from a team as a code owner July 16, 2025 06:16
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 16, 2025

Codecov Report

Attention: Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 78.87%. Comparing base (184f8fe) to head (423fa03).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
cognite/extractorutils/unstable/core/base.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #458      +/-   ##
==========================================
- Coverage   78.88%   78.87%   -0.02%     
==========================================
  Files          42       42              
  Lines        3647     3650       +3     
==========================================
+ Hits         2877     2879       +2     
- Misses        770      771       +1     
Files with missing lines Coverage Δ
cognite/extractorutils/unstable/core/base.py 76.92% <66.66%> (-0.18%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@Hmnt39 Hmnt39 left a comment

Choose a reason for hiding this comment

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

Looks good

@toondaey toondaey added the waiting-for-risk-review Waiting for a member of the risk review team to take an action label Jul 16, 2025
with extractor:
extractor.run()
"""
self._start_time = datetime.now(tz=timezone.utc)
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.

nice to set it to UTC! Have had a ton of problems correlating when the customer has some funky timezones in use when running on their systems. Very confusing.

@thorkildcognite thorkildcognite added the risk-review-ongoing Risk review is in progress label Jul 16, 2025
@thorkildcognite thorkildcognite self-assigned this Jul 16, 2025
@thorkildcognite
Copy link
Copy Markdown
Contributor

risk review ok -- it will be good to report this in the future, indeed. Today, it is quite hard to detect extractors that end up in crash-looping (we've had that with the petrel studio connector for example -- although that is a very different type of beast that isn't in Python it taught me the need for this).

@thorkildcognite thorkildcognite added waiting-for-team Waiting for the submitter or reviewer of the PR to take an action and removed waiting-for-risk-review Waiting for a member of the risk review team to take an action labels Jul 16, 2025
@toondaey
Copy link
Copy Markdown
Contributor Author

risk review ok -- it will be good to report this in the future, indeed. Today, it is quite hard to detect extractors that end up in crash-looping (we've had that with the petrel studio connector for example -- although that is a very different type of beast that isn't in Python it taught me the need for this).

This was discovered as a drawback of the previous extractors. There's a follow up PR coming soon to include this when the extractor reports a startup.

@toondaey toondaey merged commit 015ce8a into master Jul 17, 2025
8 of 9 checks passed
@toondaey toondaey deleted the set-start-time/dog-5786 branch July 17, 2025 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk-review-ongoing Risk review is in progress 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.

3 participants