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.

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

AIP-72: Task SDK support for on_task_instance_* listeners, make OpenLineage compatible #45294

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mobuchowski
Copy link
Contributor

@mobuchowski mobuchowski commented Dec 30, 2024

With AIP-72, there is no access to the database session from the worker process, and the runtime objects have some differences to the db models. This PR contains three commits that deal with that situation:

  • First commit adjusts on_task_instance_* listeners interface to AIP-72: drops session argument and makes task_instance argument an instance of RuntimeTaskInstance class, not database model
  • Second commit adds basic support for calling listeners in Task SDK, additionally adding some context fields that allow OL and other listeners not use DB.
  • Third commit adjusts OpenLineage integration to work with new interface and Task SDK.

Some followup work:

  • wrap listener calls into Activity to make logging better visible from UI, and distinct from task logs
  • add separate interface for API to capture manual/API task changes. Impossible to reuse current interface since object would be a different type (not RuntimeTaskInstance)
  • add support for more types of listeners, more in task Ensure Listeners work with Task SDK #45491

closes #45423

@boring-cyborg boring-cyborg bot added area:Executors-core LocalExecutor & SequentialExecutor area:providers area:Scheduler including HA (high availability) scheduler area:task-sdk provider:openlineage AIP-53 labels Dec 30, 2024
@mobuchowski mobuchowski force-pushed the tasksdk-call-listeners branch 4 times, most recently from f07a78e to 81b886b Compare January 2, 2025 08:20
@potiuk
Copy link
Member

potiuk commented Jan 2, 2025

@mobuchowski - can you please rebase that one -> we found and issue with @jscheffl with the new caching scheme - fixed in #45347 that would run "main" version of the tests. I am asking in all affected PRs to rebase.

@potiuk potiuk force-pushed the tasksdk-call-listeners branch from 81b886b to 15fd95a Compare January 2, 2025 12:26
@potiuk
Copy link
Member

potiuk commented Jan 2, 2025

Actually - I rebased it now.

@mobuchowski
Copy link
Contributor Author

I will rebase very soon as I'm working on some of the test failures anyway 🙂

@mobuchowski mobuchowski force-pushed the tasksdk-call-listeners branch 7 times, most recently from 35a94e2 to 86297af Compare January 6, 2025 16:35
@mobuchowski mobuchowski marked this pull request as ready for review January 6, 2025 16:52
@mobuchowski mobuchowski force-pushed the tasksdk-call-listeners branch from 86297af to 6d88389 Compare January 6, 2025 17:04
@mobuchowski mobuchowski changed the title [draft] AIP-72: call on_task_instance_* listeners AIP-72: call on_task_instance_* listeners Jan 6, 2025
@mobuchowski mobuchowski force-pushed the tasksdk-call-listeners branch 4 times, most recently from 454af61 to a7fb9e5 Compare January 8, 2025 12:32
@mobuchowski mobuchowski requested a review from potiuk as a code owner January 8, 2025 12:32
@mobuchowski mobuchowski changed the title AIP-72: call on_task_instance_* listeners AIP-72: Task SDK support for on_task_instance_* listeners, make OpenLineage compatible Jan 8, 2025
@mobuchowski mobuchowski force-pushed the tasksdk-call-listeners branch from a7fb9e5 to 0b6cd12 Compare January 8, 2025 14:05
@mobuchowski mobuchowski force-pushed the tasksdk-call-listeners branch from 3375938 to 843f34e Compare January 8, 2025 18:32
Signed-off-by: Maciej Obuchowski <obuchowski.maciej@gmail.com>
Signed-off-by: Maciej Obuchowski <obuchowski.maciej@gmail.com>
Signed-off-by: Maciej Obuchowski <obuchowski.maciej@gmail.com>
Signed-off-by: Maciej Obuchowski <obuchowski.maciej@gmail.com>
@mobuchowski mobuchowski force-pushed the tasksdk-call-listeners branch from 843f34e to 498d80c Compare January 9, 2025 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Executors-core LocalExecutor & SequentialExecutor area:providers area:Scheduler including HA (high availability) scheduler area:task-sdk provider:openlineage AIP-53
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ensure Task Instance Listeners work with Task SDK
3 participants