Skip to content

Conversation

@nadavtzaysler
Copy link

Description

changed the python client to send heartbeats to the api in order to check if the sent query is still running
solves the issue #554

( x) This is not user-visible or docs only and no release notes are required.

…ss queries that will take more than 15 minutes
@cla-bot cla-bot bot added the cla-signed label Jul 20, 2025
@RoeyoOgen RoeyoOgen requested review from ebyhr, hovaesco and mdesmet and removed request for hovaesco July 29, 2025 06:14
@gertjanal
Copy link

Instead of a heartbeat, I would opt for a callback with the status of the query so I can calculate the progress percentage every time the lastest status is received during the while not finished loop

@ebyhr ebyhr removed their request for review August 18, 2025 04:05
@jayceslesar
Copy link

any updates on this? entirely blocking me implementing this client anywhere in my orgs stack

Comment on lines +894 to +899

def _heartbeat_loop(self):
while all([not self._heartbeat_stop_event.is_set(), not self.finished, not self.cancelled,
self._heartbeat_enabled]):
if self._next_uri is None:
break

Choose a reason for hiding this comment

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

Suggested change
def _heartbeat_loop(self):
while all([not self._heartbeat_stop_event.is_set(), not self.finished, not self.cancelled,
self._heartbeat_enabled]):
if self._next_uri is None:
break
def _heartbeat_loop(self):
while all([not self._heartbeat_stop_event.is_set(), self.running,
self._heartbeat_enabled]):
if self._next_uri is None:
break

legacy_primitive_types: bool = False,
fetch_mode: Literal["mapped", "segments"] = "mapped"
fetch_mode: Literal["mapped", "segments"] = "mapped",
heartbeat_interval: float = 60.0, # seconds

Choose a reason for hiding this comment

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

nit: this could be a timedelta objector imo better to just name it heartbeat_interval_seconds

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

Development

Successfully merging this pull request may close these issues.

4 participants