SameerMesiah97 commented on code in PR #70771:
URL: https://github.com/apache/airflow/pull/70771#discussion_r3941695436
##########
airflow-ctl/src/airflowctl/api/operations.py:
##########
@@ -326,6 +329,39 @@ def delete_queued_event(self, dag_id: str, asset_id: str)
-> str | ServerRespons
self.client.delete(f"dags/{dag_id}/assets/{asset_id}/queuedEvents")
return asset_id
+ def list_state_store(self, asset_id: str) ->
AssetStateStoreCollectionResponse | ServerResponseError:
+ """List all state store entries for an asset."""
+ return super().execute_list(
+ path=f"/assets/{asset_id}/state-store",
data_model=AssetStateStoreCollectionResponse
+ )
+
+ def get_state_store(self, asset_id: str, key: str) ->
AssetStateStoreResponse | ServerResponseError:
+ """Get a single asset state store entry."""
+ self.response = self.client.get(f"assets/{asset_id}/state-store/{key}")
+ return
AssetStateStoreResponse.model_validate_json(self.response.content)
+
+ def set_state_store(self, asset_id: str, key: str, value: str) -> str |
ServerResponseError:
+ """Set an asset state store value. Creates or overwrites the key."""
+ try:
+ parsed_value = json.loads(value)
Review Comment:
Is the implicit JSON coercion here intentional? Values such as `"123"`,
`"true"`, and `"null"` will be stored as an integer, boolean, and null
respectively, whereas `"hello"` remains a string. This could be surprising to
users expecting the CLI argument to be preserved as text. If supporting JSON
values is intended, could this behaviour be made clear in the command help?
##########
airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py:
##########
@@ -56,6 +56,11 @@ def date_param():
"assets list",
"assets get 1",
"assets create-event --asset-id=1",
+ "assets set-state-store 1 test_key test_value",
+ "assets get-state-store 1 test_key",
+ "assets list-state-store 1",
+ "assets delete-state-store 1 test_key",
+ "assets clear-state-store 1",
Review Comment:
The integration command sequence deletes `test_key` before running
`clear-state-store`, so the clear command is only exercised against an
already-empty state store. Could we set another key before clearing it so that
the command is tested with an existing entry?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
