Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixing deferred operator handling of marking task as failed (#200) #3885

Merged
merged 6 commits into from
Aug 22, 2024
Merged

Fixing deferred operator handling of marking task as failed (#200) #3885

merged 6 commits into from
Aug 22, 2024

Conversation

masipauskas
Copy link
Contributor

Notable fixes:

  • Deferred and non-deferred operator now cancels running jobs on Armada if job has been marked as cancelled.
  • xcom data now contains lookout URL, therefore on-failure / on-success hooks can access it for external alerting.
  • job_request is now rendered by python renderer in Airflow, therefore ArmadaJobSubmitRequest is formatted, when rendered in Airflow UI.
  • Using Airflow serialization/deserialization for state transfer between Operator and Trigger.

Nitpick improvements:

  • Airflow operator unit tests, have been migrated to use pytest, to be consistent with Armada python client.

robertdavidsmith
robertdavidsmith reviewed Aug 21, 2024
View reviewed changes
third_party/airflow/armada/hooks.py Outdated

return RunningJobContext(queue, job.job_id, job_set_id, None)

def update_context(
Copy link
Collaborator

@robertdavidsmith robertdavidsmith Aug 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Rename update_context to refresh_context?

robertdavidsmith
robertdavidsmith reviewed Aug 21, 2024
View reviewed changes
third_party/airflow/armada/model.py Outdated
def __init__(
self,
target: str,
options: Optional[Sequence[Tuple[str, Any]]] = None,
compression: Optional[grpc.Compression] = None,
auth: Optional[grpc.AuthMetadataPlugin] = None,
auth_details: Optional[Dict[str, Any]] = None,
# auth_details: Optional[Dict[str, Any]] = None,
Copy link
Collaborator

@robertdavidsmith robertdavidsmith Aug 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Delete rather than comment?

@masipauskas masipauskas enabled auto-merge (squash) August 22, 2024 13:54
@masipauskas masipauskas merged commit 9b6f770 into master Aug 22, 2024
28 checks passed
@masipauskas masipauskas deleted the sendToGitHub/airflow-deferred-operator branch August 22, 2024 14:03
@svc-gh-ghzonetrans-p svc-gh-ghzonetrans-p restored the sendToGitHub/airflow-deferred-operator branch August 22, 2024 14:07
@masipauskas masipauskas deleted the sendToGitHub/airflow-deferred-operator branch August 22, 2024 15:24
@svc-gh-ghzonetrans-p svc-gh-ghzonetrans-p restored the sendToGitHub/airflow-deferred-operator branch August 22, 2024 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robertdavidsmith robertdavidsmith robertdavidsmith approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@masipauskas @robertdavidsmith