Skip to content

Conversation

@giffels
Copy link
Member

@giffels giffels commented Aug 11, 2025

  • Extend AsyncCacheMap to grant r/o access to cached data
  • Implement function to HTCondor Collector machines
  • Implement function to get DaemonStartTime of HTCondor Master daemon
  • Take into account DaemonStartTime when updating machine status
  • Add new mocked executor utility to support multiple consectuive responses to executor.run_command calls
  • Add tests for new features

Fixes #376

@giffels giffels added the enhancement New feature or request label Aug 11, 2025
@codecov-commenter
Copy link

codecov-commenter commented Aug 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.05%. Comparing base (58b66e0) to head (973f49a).
⚠️ Report is 58 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #378      +/-   ##
==========================================
+ Coverage   98.94%   99.05%   +0.11%     
==========================================
  Files          55       55              
  Lines        2266     2338      +72     
==========================================
+ Hits         2242     2316      +74     
+ Misses         24       22       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@giffels giffels force-pushed the add/handle_htcondor_collector_restarts branch from 30aba51 to f556898 Compare August 12, 2025 15:17
@giffels giffels force-pushed the add/handle_htcondor_collector_restarts branch from f556898 to f276b73 Compare August 13, 2025 10:03
@giffels
Copy link
Member Author

giffels commented Aug 13, 2025

For the time being, the new mocked executor utility is called mock_executor_run_command_new. I will open a new issue to refactor other adapters that currently use mock_executor_run_command. Once that is done, mock_executor_run_command_new will replace the old mock_executor_run_command as the standard utility.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really part of the feature, but fixing Type Hints is always good when visting the code anyhow.

@giffels giffels force-pushed the add/handle_htcondor_collector_restarts branch from 77795dd to 887f152 Compare August 13, 2025 15:29
@giffels giffels force-pushed the add/handle_htcondor_collector_restarts branch from 887f152 to f0d3906 Compare August 13, 2025 15:35
@giffels giffels requested review from a team, eileen-kuehn and maxfischer2781 and removed request for a team August 13, 2025 15:52
@giffels
Copy link
Member Author

giffels commented Aug 13, 2025

Stil in draft mode, but reviewers are already invited to comment about the implementation. :-)

@giffels giffels force-pushed the add/handle_htcondor_collector_restarts branch from 3c442f5 to ac9cac3 Compare August 14, 2025 09:19
@giffels giffels force-pushed the add/handle_htcondor_collector_restarts branch from ac9cac3 to 3b27beb Compare August 14, 2025 09:21
Comment on lines 87 to 88
and self._update_coroutine_receives_cache
== other._update_coroutine_receives_cache
Copy link
Member

Choose a reason for hiding this comment

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

This is extremely specific now. I guess you can just check for identity as return self is other (which then also doesn't need the type check).

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed that as well. Thanks for spotting this.

@giffels giffels force-pushed the add/handle_htcondor_collector_restarts branch from 2ed78bb to 10b7169 Compare September 12, 2025 14:31
@giffels giffels marked this pull request as ready for review September 24, 2025 15:10
maxfischer2781
maxfischer2781 previously approved these changes Oct 1, 2025
Copy link
Member

@maxfischer2781 maxfischer2781 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 to me. I've got a general maintenance comment, but it doesn't have to be forced into this PR.

@giffels giffels requested a review from maxfischer2781 October 1, 2025 16:10
Copy link
Member

@maxfischer2781 maxfischer2781 left a comment

Choose a reason for hiding this comment

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

Just realised I let this sit for a while. Thanks for the updates, LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gracefully handle HTCondor Collector restarts

3 participants