(1.x) Hightouch task hangs when it reaches a finished state other than "success"

dylanbhughes commented 1 year ago

First check

[X] I added a descriptive title to this issue.
[X] I used the GitHub search to find a similar issue and didn't find it.
[X] I searched the Prefect documentation for this issue.
[X] I checked that this issue is related to Prefect and not one of its dependencies.

Bug summary

The HightouchRunSync Task doesn't take finished sync run states other than "successful" into account.

See https://github.com/PrefectHQ/prefect/blob/1.x/src/prefect/tasks/hightouch/hightouch_utils.py#L93

while not max_wait_time or elapsed_wait_time <= max_wait_time:

                    sync_status_response = self.get_sync_run_status(sync_id=sync_id)
                    sync_status = sync_status_response["sync"]["sync_status"]

                    if sync_status == "success":
                        return sync_status_response
                    else:
                        time.sleep(wait_time_between_api_calls)
                        elapsed_wait_time += wait_time_between_api_calls

In the snippet above, the API may return "disabled", "cancelled", "failed", "success", "warning", "reporting", or "interrupted" end states. With the current logic, the Task hangs until it reaches its max_wait_time, at which point it fails.

Furthermore, it looks like the HightouchClient might be using a non-public or deprecated version of the Hightouch API?

Reproduction

from prefect import task, Flow
from prefect.tasks.hightouch.hightouch_tasks import HightouchRunSync

with Flow("Hightouch Test Flow") as flow:

    sync_result = HightouchRunSync(
        name="My Sync that Results in Warning",
        sync_id=68213,
        wait_for_completion=True,
        max_wait_time=600,
        wait_time_between_api_calls=10,
    )(api_key=HIGHTOUCH_API_KEY)

Error

No response

Versions

1.x

Additional context

No response

serinamarie commented 1 year ago

Thanks for the issue! I agree that the HightouchRunSync task could handle all terminal status codes so that it does not continue to wait after sync completion. I think someone could implement it similarly to how it's done in prefect-census and used the latest Hightouch API reference documentation to update the endpoints in the process.

cicdw commented 3 months ago

Closing as stale and because Prefect 1.x is no longer under active development.

PrefectHQ / prefect