potiuk commented on code in PR #70249:
URL: https://github.com/apache/airflow/pull/70249#discussion_r3773296937
##########
airflow-ctl/src/airflowctl/ctl/cli_config.py:
##########
@@ -205,6 +205,16 @@ def json_dict_type(val: str | dict[str, Any]) -> dict[str,
Any]:
return parsed
+def iso_datetime_type(val: str | datetime.datetime) -> datetime.datetime:
+ """Parse an ISO-8601 datetime argument."""
+ if isinstance(val, datetime.datetime):
+ return val
+ try:
+ return datetime.datetime.fromisoformat(val)
Review Comment:
`fromisoformat()` only accepts a trailing `Z` from Python 3.11 onwards, and
`airflow-ctl` supports `>=3.10`. So `--start-date 2026-07-01T12:00:00Z` parses
on 3.11+ and raises on 3.10 — same command, same server, different interpreter.
Since `Z` is the form the API itself emits, users will paste it back in:
```suggestion
return datetime.datetime.fromisoformat(val.replace("Z", "+00:00") if
val.endswith("Z") else val)
```
##########
airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py:
##########
@@ -364,6 +366,31 @@ def test_json_dict_type_rejects_non_object_json(self,
value):
with pytest.raises(argparse.ArgumentTypeError, match="expected JSON
object"):
json_dict_type(value)
+ def test_iso_datetime_type_returns_datetime_input_unchanged(self):
+ """A datetime.datetime input is returned as-is without re-parsing."""
+ import datetime
Review Comment:
`datetime` is imported at module scope (line 21) by this same PR, so this
local import can go.
##########
airflow-ctl/src/airflowctl/ctl/cli_config.py:
##########
@@ -637,7 +647,7 @@ def _python_type_from_string(type_name: str | type) -> type
| Callable:
"tuple": tuple,
"set": set,
"datetime.date": datetime.date,
Review Comment:
Same bug, one line up: `datetime.date("2026-07-01")` raises `TypeError` just
like `datetime.datetime` did. Latent today because the live date params are
served by a hand-written command, but the next generated `datetime.date`
parameter inherits it. Worth fixing in the same PR:
```suggestion
"datetime.date": iso_date_type,
"datetime.datetime": iso_datetime_type,
```
…with an `iso_date_type()` alongside `iso_datetime_type()` using
`datetime.date.fromisoformat()`.
--
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]
