Repository navigation
Conversation
b500a52 to
0ced540
Compare
| return True | ||
|
|
||
| def execute(self, context: Context) -> Any: | ||
| if not self.deferrable or self.response_check: |
There was a problem hiding this comment.
Seems like the self.response_check behvaior is being dropped? Is that an issue?
There was a problem hiding this comment.
As I understand it, this condition causes the behavior reported in #40209 because setting response_check forces the sensor onto the synchronous path. The check itself is still evaluated by self.poke(context) before deferral and by execute_complete() after the trigger returns a response. A false result defers the task again.
| :param extra_options: Additional kwargs to pass when creating a request. | ||
| For example, ``run(json=obj)`` is passed as ``aiohttp.ClientSession().get(json=obj)`` | ||
| :param poke_interval: Time to sleep using asyncio | ||
| :param initial_delay: Time to sleep before the first request. Used when the |
There was a problem hiding this comment.
Is this a pattern that is being used elsewhere (initial_delay, that is)?
There was a problem hiding this comment.
I could not find another trigger using the same initial_delay parameter. I added it because a failed response_check creates a new trigger, which would otherwise send its first request immediately instead of respecting poke_interval.
Since this introduces a new trigger parameter, I would be happy to continue discussing in this PR whether keeping it local to HttpSensorTrigger is appropriate or whether a different design would be preferable.
081173e to
efc6cf6
Compare
Co-authored-by: Jake McGrath <116606359+jroachgolf84@users.noreply.github.com>
Co-authored-by: Jake McGrath <116606359+jroachgolf84@users.noreply.github.com>
efc6cf6 to
4701b46
Compare
|
Hello @yuseok89 - thank you for your contributions to Apache Airflow! The Airflow community has introduced a limit of 5 open pull requests at a time for contributors without write access to the repository. You currently have 11 open pull requests, so - as a one-time step of introducing the limit - we closed the ones where maintainers have not engaged yet:
These pull requests stay open because maintainers are already engaged in them - they count towards your limit:
This is not a judgement of you or of your changes. We never told contributors before that opening many pull requests at once was a problem, so there is nothing to feel bad about - and nothing is lost: your branches, commits and the review history stay where they are. What we ask you to do is to make your first prioritization decision: choose which of the pull requests above matter most to you, and reopen them (up to 5 open at a time, including the ones still open) with the "Reopen pull request" button or While your pull requests are waiting for review, the most valuable thing you can do is help in other ways - reviewing other contributors' pull requests, helping with issues, and taking part in the discussions on the devlist and Slack. Why we introduced the limit, what it means for you and how to reopen or restore a pull request is explained in https://github.com/apache/airflow/blob/main/contributing-docs/32_open_pull_request_limit.rst. Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting |
closes: #40209
HttpSensor(deferrable=True)with aresponse_checksilently fell back to the synchronous poke loop, occupying a worker slot for the whole wait. It now defers like any other deferrable sensor: the triggerer waits for the endpoint to respond without error, and the check runs on the worker. If the check returnsFalse, the task defers again while preserving the original sensor timeout andpoke_intervalpacing.The callable stays on the worker. The trigger serializes and returns the HTTP response only when a
response_checkis configured; otherwise, it preserves the existing lightweight success event. This follows the same general pattern thatS3KeySensoruses forcheck_fn.Verified with a live Dag
An
HttpSensor(deferrable=True, poke_interval=20)polling this Airflow's own api-server, with aresponse_checkthat only passes two minutes after the run was triggered.Before the change the task never left
running, since aresponse_checkforced it onto the synchronous path. After the change it entersdeferredafter the initial worker-side poke, wakes every 20 seconds to evaluate the check on the worker, and succeeds once the check passes:Was generative AI tooling used to co-author this PR?
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.