Enhance HexRunProjectOperator and HexHook #21
Conversation
jacobcbeaudin
force-pushed
the
jacobcbeaudin/enhance-hex-operator
branch
5 times, most recently
from
fc65f1e to
7e2a80b
Compare
- Remove deprecated "apply_default" decorator from HexRunProjectOperator - Implement retry mechanism for API polling in HexHook - Decouple polling logic from run_and_poll method - Add max_poll_retries and poll_retry_delay parameters - Update flake8 pre-commit hook to use GitHub repository - Update pre-commit hooks to latest versions
jacobcbeaudin
force-pushed
the
jacobcbeaudin/enhance-hex-operator
branch
from
7e2a80b to
853e83b
Compare
| if project_status not in VALID_STATUSES: | ||
| raise AirflowException(f"Unhandled status: {project_status}") |
There was a problem hiding this comment.
I don't think we want to remove this exception
| json=mock_run, | ||
| json={"projectId": "abc-123", "runId": "1"}, |
| endpoint = f"api/v1/project/{project_id}/run/{run_id}" | ||
| method = "DELETE" | ||
|
|
||
| self.run(method=method, endpoint=endpoint) | ||
| return run_id | ||
|
|
||
| def run_and_poll( | ||
| def run_status_with_retries( | ||
| self, project_id: str, run_id: str, max_retries: int = 3, retry_delay: int = 1 |
There was a problem hiding this comment.
A retry limit should be enforced server-side to prevent the caller from specifying an excessive number of retries for project runs, which could lead to resource exhaustion or unintended behavior.
There was a problem hiding this comment.
Is implementing a server-side retry limit currently prioritized? As this PR focuses on client-side Airflow hooks, I can hardcode a conservative number of retries and delay to prevent configurability. This would address immediate concerns while keeping the implementation on the client side. Let me know if you'd like me to proceed with this approach.
There was a problem hiding this comment.
Sorry for the delay here! (Also this is a product-manager translation from an engineer, so apologies if I lose something in translation!)
We rate limit requests, so at this time don't intend to implement a server-side retry limit. Also, our API is fairly non-flakey, so we don't think users will need to retry that often.
That being said, it would still be great to hardcode a conservative number of retries juuuust in case — 3 seems like a good spot!
Enhance HexRunProjectOperator and HexHook
This PR addresses issues #19 and #20, significantly improving the functionality and reliability of the HexRunProjectOperator and HexHook. Most importantly, it mitigates the issue where jobs were incorrectly marked as failed due to Hex API failures, not actual job failures.
Key Improvements
apply_defaultdecorator from HexRunProjectOperator (Fixes Remove deprecated apply_default decorator from HexOperator #19)make initcommandDetailed Changes
1. Prevent false job failures
2. Remove deprecated
apply_defaultdecorator@apply_defaultdecorator from the HexRunProjectOperator class3. Implement retry mechanism for API polling
max_poll_retriesandpoll_retry_delaycontrol retry behavior4. Decouple polling logic
5. New parameters
max_poll_retries: Maximum number of retries for a failed poll (default: 3)poll_retry_delay: Delay between retries in seconds (default: 5)6. Update pre-commit hooks
https://github.com/pycqa/flake8instead of the GitLab repositoryTesting
Checklist
This update significantly improves the reliability of the Hex Airflow provider by ensuring that jobs are not incorrectly marked as failed due to API communication issues. Users will now have a more accurate representation of their job statuses, reducing false alarms and improving overall system dependability.