You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "jx2lee (via GitHub)" <gi...@apache.org> on 2025/06/02 15:17:29 UTC
[I] airflowctl: action_on_existence flag returns empty success & errors arrays in variables import [airflow]
jx2lee opened a new issue, #51310:
URL: https://github.com/apache/airflow/issues/51310
### Apache Airflow version
3.0.1
### If "Other Airflow 2 version" selected, which one?
_No response_
### What happened?
related: #50908
details: https://github.com/apache/airflow/pull/50908#issuecomment-2906582468
cc. @bugraoz93
### What you think should happen instead?
_No response_
### How to reproduce
When importing variables with `--action_on_existence` flag set to **fail**, **overwrite**, or **skip**,
the API responds `200 OK`, but both `results.success` and `results.errors` are empty.
This makes it impossible to determine whether the existing variables were handled as expected.
### Operating System
osx
### Versions of Apache Airflow Providers
_No response_
### Deployment
Other
### Deployment details
_No response_
### Anything else?
### Reproduction Steps
1. Pre-condition: Two variables already exist in Airflow
```json
[
{"key": "foo", "value": "1"},
{"key": "bar", "value": "2"}
]
```
2. Prepare import file (vars.json) containing the same keys:
```json
[
{"key": "foo", "value": "99"},
{"key": "bar", "value": "88"}
]
```
3. Run CLI:
```bash
airflow variables import vars.json --action_on_existence fail
```
You'll found empty success/errors.
### Are you willing to submit PR?
- [x] Yes I am willing to submit a PR!
### Code of Conduct
- [x] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
--
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.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] airflowctl: action_on_existence flag returns empty success & errors in `variables import` [airflow]
Posted by "jx2lee (via GitHub)" <gi...@apache.org>.
jx2lee commented on issue #51310:
URL: https://github.com/apache/airflow/issues/51310#issuecomment-2953670874
> We always send create action, so it might be good to only print success and errors rather than for all actions. We can bring those functionalities in future releases such as in place updates. It doesn't need to be particularly in the current version
Thanks for sharing your view! I agreed, make PR soon. 👍🏽
--
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: [I] airflowctl: action_on_existence flag returns empty success & errors in `variables import` [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on issue #51310:
URL: https://github.com/apache/airflow/issues/51310#issuecomment-2952588125
> If we'll update methods to return BulkResponse, how about implementing it as follows?
>
> screencast.2025-06-07.21-53-43.mp4
Sure, great catch @jx2lee! Thanks! Feel free to create a PR. We can indeed use the same model in airflowctl and fix this one. It would be easier to discuss over the code :) You did the change in operations right?
--
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: [I] airflowctl: action_on_existence flag returns empty success & errors in `variables import` [airflow]
Posted by "jx2lee (via GitHub)" <gi...@apache.org>.
jx2lee commented on issue #51310:
URL: https://github.com/apache/airflow/issues/51310#issuecomment-2952649682
> You did the change in operations right?
Sure. I did change in operation, to BulkResponse. Once the output format is defined or guide is provided, I can quickly make it.
Below pseudo code.
```
diff --git a/airflow-ctl/src/airflowctl/api/operations.py b/airflow-ctl/src/airflowctl/api/operations.py
index 00f7601348..485d157ad3 100644
--- a/airflow-ctl/src/airflowctl/api/operations.py
+++ b/airflow-ctl/src/airflowctl/api/operations.py
@@ -33,6 +33,7 @@ from airflowctl.api.datamodels.generated import (
BackfillCollectionResponse,
BackfillPostBody,
BackfillResponse,
+ BulkResponse,
BulkActionResponse,
BulkBodyConnectionBody,
BulkBodyPoolBody,
@@ -676,7 +677,7 @@ class VariablesOperations(BaseOperations):
"""CRUD multiple variables."""
try:
self.response = self.client.patch("variables", json=variables.model_dump())
- return BulkActionResponse.model_validate_json(self.response.content)
+ return BulkResponse.model_validate_json(self.response.content)
except ServerResponseError as e:
raise e
```
```
diff --git a/airflow-ctl/src/airflowctl/ctl/commands/variable_command.py b/airflow-ctl/src/airflowctl/ctl/commands/variable_command.py
index 1c31828a37..b69c3ae0ab 100644
--- a/airflow-ctl/src/airflowctl/ctl/commands/variable_command.py
+++ b/airflow-ctl/src/airflowctl/ctl/commands/variable_command.py
@@ -35,7 +35,8 @@ from airflowctl.api.datamodels.generated import (
@provide_api_client(kind=ClientKind.CLI)
def import_(args, api_client=NEW_API_CLIENT):
"""Import variables from a given file."""
- success_message = "[green]Import successful! success: {success}, errors: {errors}[/green]"
+ success_message = "[green]Import successful! success: {success}[/green]"
+ errors_message = "[red]But errors occured, errors: {errors}[/red]"
if not os.path.exists(args.file):
rich.print(f"[red]Missing variable file: {args.file}")
sys.exit(1)
@@ -71,8 +72,10 @@ def import_(args, api_client=NEW_API_CLIENT):
]
)
result = api_client.variables.bulk(variables=bulk_body)
- rich.print(success_message.format(success=result.success, errors=result.errors))
- return result.success, result.errors
+ rich.print(success_message.format(success=result.create.success))
+ if result.create.errors:
+ rich.print(errors_message.format(errors=result.create.errors))
+ return result.create.success, result.create.errors
```
--
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: [I] airflowctl: action_on_existence flag returns empty success & errors in `variables import` [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 closed issue #51310: airflowctl: action_on_existence flag returns empty success & errors in `variables import`
URL: https://github.com/apache/airflow/issues/51310
--
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: [I] airflowctl: action_on_existence flag returns empty success & errors in `variables import` [airflow]
Posted by "jx2lee (via GitHub)" <gi...@apache.org>.
jx2lee commented on issue #51310:
URL: https://github.com/apache/airflow/issues/51310#issuecomment-2952424007
@bugraoz93 I found potential problem!
Airflow REST API documentation indicates that bulk variables endpoint returns a [BulkResponse](https://github.com/apache/airflow/blob/1307f568ab664a679b67bfebc1b95f4490067e14/airflow-ctl/src/airflowctl/api/datamodels/generated.py#L153). However, bulk method in VariableOperation within airflowctl currently returns [BulkActionResponse](https://github.com/apache/airflow/blob/1307f568ab664a679b67bfebc1b95f4490067e14/airflow-ctl/src/airflowctl/api/operations.py#L675). This discrepancy could lead to inconsistencies in handling API responses.
REST API response bulk variables.
This diff might also exist in `PoolOperation`'s bulk method.
Should we consider updating these methods to return `BulkResponse` to align with the API spec? If you can provide guidance or direction, I'd be happy to create a PR to address this.
--
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: [I] airflowctl: action_on_existence flag returns empty success & errors in `variables import` [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on issue #51310:
URL: https://github.com/apache/airflow/issues/51310#issuecomment-2932084072
I think testing with Postman and seeing if it is working for all bulk operations should be enough. Thanks for creating it! I assigned to you
--
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: [I] airflowctl: action_on_existence flag returns empty success & errors in `variables import` [airflow]
Posted by "jx2lee (via GitHub)" <gi...@apache.org>.
jx2lee commented on issue #51310:
URL: https://github.com/apache/airflow/issues/51310#issuecomment-2952455591
If we'll update methods to return BulkResponse, how about implementing it as follows?
https://github.com/user-attachments/assets/146a0016-3f5a-4154-9c2e-a7340b8a82cb
--
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: [I] airflowctl: action_on_existence flag returns empty success & errors arrays in variables import [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on issue #51310:
URL: https://github.com/apache/airflow/issues/51310#issuecomment-2931222748
Thanks for opening your first issue here! Be sure to follow the issue template! If you are willing to raise PR to address this issue please do so, no need to wait for approval.
--
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: [I] airflowctl: action_on_existence flag returns empty success & errors in `variables import` [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on issue #51310:
URL: https://github.com/apache/airflow/issues/51310#issuecomment-2952726890
> > You did the change in operations right?
>
> @bugraoz93 Sure. I did change in operation, to BulkResponse. Once the output format is defined or guide is provided, I can quickly make it.
> Below pseudo code.
> ```
> diff --git a/airflow-ctl/src/airflowctl/api/operations.py b/airflow-ctl/src/airflowctl/api/operations.py
> index 00f7601348..485d157ad3 100644
> --- a/airflow-ctl/src/airflowctl/api/operations.py
> +++ b/airflow-ctl/src/airflowctl/api/operations.py
> @@ -33,6 +33,7 @@ from airflowctl.api.datamodels.generated import (
> BackfillCollectionResponse,
> BackfillPostBody,
> BackfillResponse,
> + BulkResponse,
> BulkActionResponse,
> BulkBodyConnectionBody,
> BulkBodyPoolBody,
> @@ -676,7 +677,7 @@ class VariablesOperations(BaseOperations):
> """CRUD multiple variables."""
> try:
> self.response = self.client.patch("variables", json=variables.model_dump())
> - return BulkActionResponse.model_validate_json(self.response.content)
> + return BulkResponse.model_validate_json(self.response.content)
> except ServerResponseError as e:
> raise e
> ```
>
> ```
> diff --git a/airflow-ctl/src/airflowctl/ctl/commands/variable_command.py b/airflow-ctl/src/airflowctl/ctl/commands/variable_command.py
> index 1c31828a37..b69c3ae0ab 100644
> --- a/airflow-ctl/src/airflowctl/ctl/commands/variable_command.py
> +++ b/airflow-ctl/src/airflowctl/ctl/commands/variable_command.py
> @@ -35,7 +35,8 @@ from airflowctl.api.datamodels.generated import (
> @provide_api_client(kind=ClientKind.CLI)
> def import_(args, api_client=NEW_API_CLIENT):
> """Import variables from a given file."""
> - success_message = "[green]Import successful! success: {success}, errors: {errors}[/green]"
> + success_message = "[green]Import successful! success: {success}[/green]"
> + errors_message = "[red]But errors occured, errors: {errors}[/red]"
> if not os.path.exists(args.file):
> rich.print(f"[red]Missing variable file: {args.file}")
> sys.exit(1)
> @@ -71,8 +72,10 @@ def import_(args, api_client=NEW_API_CLIENT):
> ]
> )
> result = api_client.variables.bulk(variables=bulk_body)
> - rich.print(success_message.format(success=result.success, errors=result.errors))
> - return result.success, result.errors
> + rich.print(success_message.format(success=result.create.success))
> + if result.create.errors:
> + rich.print(errors_message.format(errors=result.create.errors))
> + return result.create.success, result.create.errors
> ```
We always send create action, so it might be good to only print success and errors rather than for all actions. We can bring those functionalities in future releases such as in place updates. It doesn't need to be particularly in the current version
--
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