Rearrange HTEX hold_worker because it holds managers, not workers #3840
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
See issue #3839 This changes the Python API side of this method to reflect that the action is on managers, not workers. A separate PR will change the wire protocol command.
khk-globus
reviewed
Apr 15, 2025
khk-globus
reviewed
Apr 15, 2025
Comment on lines
658
to
+659
| logger.debug("Sending hold to manager: {}".format(manager['manager'])) | ||
| self.hold_worker(manager['manager']) | ||
| self._hold_manager(manager['manager']) |
There was a problem hiding this comment.
Unrelated to the thrust of this PR, but I see it now so observing it: since this is a blocking request, too bad this loop isn't in bulk. Or that _hold_manager is blocking.
There was a problem hiding this comment.
yeah, that's probably quite interesting especially for large-node-count blocks (eg 1000 nodes). I'll link this comment from #3368 which talks about reworking this protocol.
khk-globus
approved these changes
Apr 15, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See issue #3839
This changes the Python API side of this method to reflect that the action is on managers, not workers.
A separate PR will change the wire protocol command.
Changed Behaviour
this is internal to htex - nothing except stack traces should change for users
Type of change