You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "jroachgolf84 (via GitHub)" <gi...@apache.org> on 2025/11/16 02:51:47 UTC
[PR] `issue-58718`: Adding Task SDK integration tests for `Variable` operations [airflow]
jroachgolf84 opened a new pull request, #58357:
URL: https://github.com/apache/airflow/pull/58357
## Description
Added integration tests for `Variable` operations. There were two stubbed tests in `test_variable_operations.py`. These were:
* `test_variable_set`
* `test_variable_delete`
The pattern implemented in `test_xcom_operations.py` was leveraged as a reference for implementing these stubbed tests, especially `test_variable_delete`. These tests can be run by pulling down this branch and running the command below:
```
breeze testing task-sdk-integration-tests tests/task_sdk_tests/test_variable_operations.py
```
## Outstanding Issues
When running the `test_variable_operations.py::test_variable_delete` function, the following is being output (see below). *This implies that the variable is NOT being truly being deleted when `sdk_client.variables.delete` is called.*
```
# Validate that the Variable has in fact been deleted
get_response = sdk_client.variables.get("test_variable_key")
assert isinstance(get_response, ErrorResponse)
AssertionError: assert False
+ where False = isinstance(VariableResponse(key='test_variable_key', value='test_variable_value'), ErrorResponse)
```
related: https://github.com/apache/airflow/issues/58178#issuecomment-3521015783
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] `issue-58718`: Adding Task SDK integration tests for `Variable` operations [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on code in PR #58357:
URL: https://github.com/apache/airflow/pull/58357#discussion_r2534074347
##########
task-sdk-integration-tests/tests/task_sdk_tests/test_variable_operations.py:
##########
@@ -76,25 +76,63 @@ def test_variable_get_not_found(sdk_client):
console.print("[green]✅ Variable get (not found) test passed!")
-@pytest.mark.skip(reason="TODO: Implement Variable set test")
def test_variable_set(sdk_client):
"""
Test setting variable value.
Expected: OKResponse with ok=True
Endpoint: PUT /execution/variables/{key}
"""
- console.print("[yellow]TODO: Implement test_variable_set")
- raise NotImplementedError("test_variable_set not implemented")
+ console.print("[yellow]TODO: Setting variable...")
Review Comment:
Addressed!
##########
task-sdk-integration-tests/tests/task_sdk_tests/test_variable_operations.py:
##########
@@ -76,25 +76,63 @@ def test_variable_get_not_found(sdk_client):
console.print("[green]✅ Variable get (not found) test passed!")
-@pytest.mark.skip(reason="TODO: Implement Variable set test")
def test_variable_set(sdk_client):
"""
Test setting variable value.
Expected: OKResponse with ok=True
Endpoint: PUT /execution/variables/{key}
"""
- console.print("[yellow]TODO: Implement test_variable_set")
- raise NotImplementedError("test_variable_set not implemented")
+ console.print("[yellow]TODO: Setting variable...")
+
+ response = sdk_client.variables.set("test_variable_key", "test_variable_value")
+
+ console.print(" Variable Set Response ".center(72, "="))
+ console.print(f"[bright_blue]Response Type:[/] {type(response).__name__}")
+ console.print(f"[bright_blue]Status:[/] {response.ok}")
+ console.print("=" * 72)
+
+ assert isinstance(response, OKResponse)
+ assert response.ok is True
+
+ console.print("[green]✅ Variable set test passed!")
-@pytest.mark.skip(reason="TODO: Implement Variable delete test")
def test_variable_delete(sdk_client):
"""
Test deleting variable value.
Expected: OKResponse with ok=True
Endpoint: DELETE /execution/variables/{key}
"""
- console.print("[yellow]TODO: Implement test_variable_delete")
- raise NotImplementedError("test_variable_delete not implemented")
+ console.print("[yellow]Deleting variable...")
+
+ key: str = "test_variable_delete_key"
Review Comment:
Addresses!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] `issue-58718`: Adding Task SDK integration tests for `Variable` operations [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh merged PR #58357:
URL: https://github.com/apache/airflow/pull/58357
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] `issue-58718`: Adding Task SDK integration tests for `Variable` operations [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on PR #58357:
URL: https://github.com/apache/airflow/pull/58357#issuecomment-3540180089
Hello - I appreciate you trying this out, I just pulled in your branch and figured out the problem.
This is a classic case of the secrets backend taking precedence over the database.
In Docker Compose you’ve defined an environment variable for test_variable_key and secrets backends are read-only, so when your test calls Variable.set(), the value is written to the database — and when you delete it, Airflow correctly removes it from the DB.
The API server log even explains exactly what’s happening:
```python
[2025-11-17T06:12:57.057+0000] {variable.py:404} WARNING -
The variable test_variable_key is defined in the EnvironmentVariablesBackend
secrets backend, which takes precedence over reading from the database.
The value in the database will be updated, but to read it you have to
delete the conflicting variable from EnvironmentVariablesBackend.
```
So later, when your test reads the variable to confirm the deletion, it’s actually reading it from the secrets backend, not the database. That makes it appear as if the variable wasn’t deleted but it was.
Database proof:
```shell script
airflow=# SELECT key FROM variable;
key
-----
(0 rows)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] `issue-58718`: Adding Task SDK integration tests for `Variable` operations [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #58357:
URL: https://github.com/apache/airflow/pull/58357#discussion_r2534039037
##########
task-sdk-integration-tests/tests/task_sdk_tests/test_variable_operations.py:
##########
@@ -76,25 +76,63 @@ def test_variable_get_not_found(sdk_client):
console.print("[green]✅ Variable get (not found) test passed!")
-@pytest.mark.skip(reason="TODO: Implement Variable set test")
def test_variable_set(sdk_client):
"""
Test setting variable value.
Expected: OKResponse with ok=True
Endpoint: PUT /execution/variables/{key}
"""
- console.print("[yellow]TODO: Implement test_variable_set")
- raise NotImplementedError("test_variable_set not implemented")
+ console.print("[yellow]TODO: Setting variable...")
+
+ response = sdk_client.variables.set("test_variable_key", "test_variable_value")
+
+ console.print(" Variable Set Response ".center(72, "="))
+ console.print(f"[bright_blue]Response Type:[/] {type(response).__name__}")
+ console.print(f"[bright_blue]Status:[/] {response.ok}")
+ console.print("=" * 72)
+
+ assert isinstance(response, OKResponse)
+ assert response.ok is True
+
+ console.print("[green]✅ Variable set test passed!")
-@pytest.mark.skip(reason="TODO: Implement Variable delete test")
def test_variable_delete(sdk_client):
"""
Test deleting variable value.
Expected: OKResponse with ok=True
Endpoint: DELETE /execution/variables/{key}
"""
- console.print("[yellow]TODO: Implement test_variable_delete")
- raise NotImplementedError("test_variable_delete not implemented")
+ console.print("[yellow]Deleting variable...")
+
+ key: str = "test_variable_delete_key"
Review Comment:
Maybe a comment about why a new key would be good
##########
task-sdk-integration-tests/tests/task_sdk_tests/test_variable_operations.py:
##########
@@ -76,25 +76,63 @@ def test_variable_get_not_found(sdk_client):
console.print("[green]✅ Variable get (not found) test passed!")
-@pytest.mark.skip(reason="TODO: Implement Variable set test")
def test_variable_set(sdk_client):
"""
Test setting variable value.
Expected: OKResponse with ok=True
Endpoint: PUT /execution/variables/{key}
"""
- console.print("[yellow]TODO: Implement test_variable_set")
- raise NotImplementedError("test_variable_set not implemented")
+ console.print("[yellow]TODO: Setting variable...")
Review Comment:
TODO can go :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org