Uh oh!
There was an error while loading. Please reload this page.
Rework StackdriverTaskHandler for the structlog era #65191 - #65198
Conversation
Uh oh!
There was an error while loading. Please reload this page.
ashb
left a comment
There was a problem hiding this comment.
Please follow the pattern used for for the other Google logging provider -- make the *TaskHandler have a self.io property of the new Remote IO class and delegate things to it, rather than duplicating all the logic twice.
5fdf14f to
4f52dbcCompare64fe13e to
6358a98Compare@haseebmalik18 Converting to draft — this PR doesn't yet meet our Pull Request quality criteria.
See the linked criteria for how to fix each item, then mark the PR "Ready for review". This is not a rejection — just an invitation to bring the PR up to standard. No rush. Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you. |
Quick follow-up to the triage comment above — one clarification on the "Unresolved review comments" item: Once you believe a thread has been addressed — whether by pushing a fix, or by replying in-thread with an explanation of why the suggestion doesn't apply — please mark the thread as resolved yourself by clicking the "Resolve conversation" button at the bottom of each thread. Reviewers don't auto-close their own threads, so an addressed-but-unresolved thread reads as "still waiting on the author" and keeps the PR from moving forward. The author doing the resolve-click is the expected convention on this project. Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you. |
leestro
commented
Apr 28, 2026
Is the failing Static checks job anything to be concerned about? |
haseebmalik18
commented
Apr 28, 2026
I don't believe so |
haseebmalik18
commented
Apr 28, 2026
Static checks are good now |
shahar1
left a comment
There was a problem hiding this comment.
I took another look before merging, and likely found a bug + some other nits.
I'll be happy if you could revise this PR.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
44519dd to
fd80b0dComparehaseebmalik18
commented
May 13, 2026
Made the requested changes |
potiuk
commented
May 18, 2026
@haseebmalik18 — There are 1 unresolved review thread on this PR from @eladkal. Could you either push a fix or reply in each thread explaining why the feedback doesn't apply? Once you believe the feedback is addressed, mark the thread as resolved so the reviewer isn't re-pinged needlessly. Thanks! Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you. |
haseebmalik18
commented
May 23, 2026
@shahar1 Just following up on this whenever you get a chance |
shahar1
commented
May 23, 2026
LGTM, @ashb? |
ashb
commented
May 26, 2026
Taking a look now |
ashb
left a comment
There was a problem hiding this comment.
LGTM. I haven't looked in detail, but it avoids duplication and follows the good pattern. Removing my review.
Add
StackdriverRemoteLogIOwithprocessorsfor real-time log streaming,upload(), andread()methods following the same pattern as CloudWatch, GCS, S3, HDFS, and other backends migrated in #48491.closes: #65191
related: #65191