Conversation
Fix Variable.akeys: wrapping an async call inside lazy_object_proxy.Proxy is invalid because await cannot be used inside a plain lambda. The method now awaits _async_get_variable_keys directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
dabla
force-pushed
the
feature/add-async-accessors-variable
branch
from
August 31, 2026 14:43
93418a6 to
6c0e252
Compare
amoghrajesh
requested changes
Sep 15, 2026
amoghrajesh
left a comment
amoghrajesh
left a comment
Contributor
There was a problem hiding this comment.
Requesting changes to avoid accidental merge, directionally good, but there's work needed.
…e run the sync one via asyncio.to_thread
…ctly Avoid importing and reassigning AsyncMock in every test; the fixture's asend is already an AsyncMock, so tests can set return_value/side_effect on it directly, per top-level import convention. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…cMock imports Same cleanup as test_variables.py: set return_value/side_effect directly on the fixture's mock_supervisor_comms.asend instead of importing and reassigning AsyncMock per test; move json import to the top of the file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
dabla
force-pushed
the
feature/add-async-accessors-variable
branch
from
September 15, 2026 12:09
af48dc4 to
f8be671
Compare
This was referenced Sep 20, 2026
dabla
added a commit
to dabla/airflow
that referenced
this pull request
Sep 30, 2026
The error an iterated async task gets for a synchronous SDK call already points at Variable.aget/aset, which apache#72329 adds; the class docstring still said Variable had no async equivalent. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
amoghrajesh
reviewed
Sep 30, 2026
amoghrajesh
left a comment
amoghrajesh
left a comment
Contributor
There was a problem hiding this comment.
Looking better, some comments.
json is already imported at module level in context.py, and _VARIABLE_KEYS_PAGE_SIZE is now imported once at the top of each test module. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Mirror the sync VariableAccessor masking tests for _async_get_variable so _async_mask_and_deserialize_variable and the SecretCache hit path are covered. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add async Variable methods (
aget,aset,adelete,akeys) with testsThis PR adds async counterparts to all synchronous
Variablemethods.What changed
Variableclass (task-sdk/src/airflow/sdk/definitions/variable.py)Four async class-methods are now fully functional:
Variable.agetVariable.get_async_get_variable; falls back todefaultonVARIABLE_NOT_FOUNDVariable.asetVariable.set_async_set_variable; checks secrets backends for write conflictsVariable.adeleteVariable.delete_async_delete_variable; invalidates the secret cacheVariable.akeysVariable.keys_async_get_variable_keyseagerly (see bug fix below)Context helpers (
task-sdk/src/airflow/sdk/execution_time/context.py)The four underlying
_async_*functions were already implemented:_async_get_variable— checksSecretCache, iterates backends viaasyncio.to_thread, falls through to the Execution API viaasend._async_set_variable— warns on backend write conflicts viaasyncio.to_thread, sendsPutVariableviaasend, invalidates cache._async_delete_variable— sendsDeleteVariableviaasend, invalidates cache._async_get_variable_keys— paginatesGetVariableKeysrequests viaasend.Tests
test_variables.pyAdded
TestAsyncVariablesandTestAsyncVariableKeysmirroring all existingsync test cases:
test_avar_get— simple and JSON-deserialized valuestest_avar_set— plain and JSON-serialized values; verifiesPutVariablepayloadtest_avar_delete— verifiesDeleteVariableis sent viaasendtest_akeys— prefix filtering (all / with prefix / empty)test_akeys_paginates_when_results_exceed_page_sizetest_akeys_raises_on_error_responsetest_akeys_raises_on_unexpected_response_typetest_context.pyAdded
TestAsyncVariableContextwith direct unit tests for the four context helpers(previously untested):
_async_get_variable: from API, from secrets backend, not-found raises_async_set_variable: simple value, JSON-serialized value_async_delete_variable: sendsDeleteVariable_async_get_variable_keys: prefix filtering, pagination, error, unexpected responseWas generative AI tooling used to co-author this PR?
GitHub Copilot (Claude Sonnet 4.6)
{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.