This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new ffa426b64d7 airflowctl: make required CLI params positional, keep
optional as --flag (#66768)
ffa426b64d7 is described below
commit ffa426b64d7d99a66482b817887ee74d399824c4
Author: Stefan Wang <[email protected]>
AuthorDate: Tue May 12 14:57:46 2026 -0700
airflowctl: make required CLI params positional, keep optional as --flag
(#66768)
* airflowctl: make required CLI params positional, keep optional as --flag
Auto-generated commands such as ``airflowctl dags get-details`` now accept
required primitive parameters positionally:
airflowctl dags get-details my_dag_id
instead of the previous ``--dag-id my_dag_id`` form. Optional parameters
and booleans keep the ``--flag`` form.
This follows the dev-list lazy consensus on airflowctl parameter style.
A parameter is considered required when the operation method declares it
without a default and without ``| None`` in its annotation. Datamodel-
expanded body fields are unaffected — they are not "parameters of the
operation method" in this sense and continue to use ``--flag``.
* tests: tmp_path fixture for command-factory; positional form for
integration tests
Two follow-ups to the positional-required-args change:
- ``TestCommandFactory._save_temp_operations_py`` previously wrote a
shared ``test_command.py`` in cwd; under pytest-xdist that file is
raced by workers, so ``next(arg for arg in jobs_list_args if ...)``
in one test could see content written by another and raise
``StopIteration``. Helper now takes the per-test ``tmp_path`` and
returns the full path. The classmethod ``teardown_method`` that
removed the shared file is no longer needed (pytest auto-cleans
``tmp_path``).
- The Airflow CTL PROD-image integration tests still invoked converted
parameters with the old ``--flag value`` form (e.g.
``--variable-key=X``, ``--section X --option Y``,
``--dag-id=example_bash_operator``). Updated each occurrence to the
positional form that the regenerated CLI now expects. Optional
parameters (``--logical-date``, ``--run-after``, ``--is-paused``,
``--state``, ``--limit``, etc.) stay as ``--flag``.
Signed-off-by: 1fanwang <[email protected]>
---------
Signed-off-by: 1fanwang <[email protected]>
---
.../airflowctl_tests/test_airflowctl_commands.py | 60 ++++-----
.../test_config_sensitive_masking.py | 4 +-
airflow-ctl/RELEASE_NOTES.rst | 11 ++
airflow-ctl/src/airflowctl/ctl/cli_config.py | 75 +++++++++--
.../tests/airflow_ctl/ctl/test_cli_config.py | 148 ++++++++++++++++-----
5 files changed, 224 insertions(+), 74 deletions(-)
diff --git
a/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py
b/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py
index 15595269d26..b8b49ad3ae7 100644
--- a/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py
+++ b/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py
@@ -54,12 +54,12 @@ TEST_COMMANDS = [
"auth list-envs",
# Assets commands
"assets list",
- "assets get --asset-id=1",
+ "assets get 1",
"assets create-event --asset-id=1",
# Backfill commands
- "backfill list",
+ "backfill list example_bash_operator",
# Config commands
- "config get --section core --option executor",
+ "config get core executor",
"config list",
"config lint",
# Connections commands
@@ -67,62 +67,62 @@ TEST_COMMANDS = [
"connections list",
"connections list -o yaml",
"connections list -o table",
- "connections get --conn-id=test_con",
- "connections get --conn-id=test_con -o json",
+ "connections get test_con",
+ "connections get test_con -o json",
"connections update --connection-id=test_con --conn-type=postgres",
"connections import tests/airflowctl_tests/fixtures/test_connections.json",
- "connections delete --conn-id=test_con",
- "connections delete --conn-id=test_import_conn",
+ "connections delete test_con",
+ "connections delete test_import_conn",
# Dags commands
"dags list",
- "dags get --dag-id=example_bash_operator",
- "dags get-details --dag-id=example_bash_operator",
- "dags get-stats --dag-ids=example_bash_operator",
- "dags get-version --dag-id=example_bash_operator --version-number=1",
+ "dags get example_bash_operator",
+ "dags get-details example_bash_operator",
+ "dags get-stats example_bash_operator",
+ "dags get-version example_bash_operator 1",
"dags list-import-errors",
- "dags list-version --dag-id=example_bash_operator",
+ "dags list-version example_bash_operator",
"dags list-warning",
# Order of trigger and pause/unpause is important for test stability
because state checked
- "dags trigger --dag-id=example_bash_operator --logical-date={date_param}
--run-after={date_param}",
+ "dags trigger example_bash_operator --logical-date={date_param}
--run-after={date_param}",
# Test trigger without logical-date (should default to now)
- "dags trigger --dag-id=example_bash_operator",
+ "dags trigger example_bash_operator",
"dags pause example_bash_operator",
"dags unpause example_bash_operator",
# Dag Run commands
- 'dagrun get --dag-id=example_bash_operator
--dag-run-id="manual__{date_param}"',
- "dags update --dag-id=example_bash_operator --no-is-paused",
+ 'dagrun get example_bash_operator "manual__{date_param}"',
+ "dags update example_bash_operator --no-is-paused",
# Dag Run commands
"dagrun list --dag-id example_bash_operator --state success --limit=1",
# XCom commands - need a Dag run with completed tasks
- 'xcom add --dag-id=example_bash_operator
--dag-run-id="manual__{date_param}" --task-id=runme_0 --key={xcom_key}
--value=\'{{"test": "value"}}\'',
- 'xcom get --dag-id=example_bash_operator
--dag-run-id="manual__{date_param}" --task-id=runme_0 --key={xcom_key}',
- 'xcom list --dag-id=example_bash_operator
--dag-run-id="manual__{date_param}" --task-id=runme_0',
- 'xcom edit --dag-id=example_bash_operator
--dag-run-id="manual__{date_param}" --task-id=runme_0 --key={xcom_key}
--value=\'{{"updated": "value"}}\'',
- 'xcom delete --dag-id=example_bash_operator
--dag-run-id="manual__{date_param}" --task-id=runme_0 --key={xcom_key}',
+ 'xcom add example_bash_operator "manual__{date_param}" runme_0 {xcom_key}
\'{{"test": "value"}}\'',
+ 'xcom get example_bash_operator "manual__{date_param}" runme_0 {xcom_key}',
+ 'xcom list example_bash_operator "manual__{date_param}" runme_0',
+ 'xcom edit example_bash_operator "manual__{date_param}" runme_0 {xcom_key}
\'{{"updated": "value"}}\'',
+ 'xcom delete example_bash_operator "manual__{date_param}" runme_0
{xcom_key}',
# Jobs commands
"jobs list",
# Pools commands
"pools create --name=test_pool --slots=5",
"pools list",
- "pools get --pool-name=test_pool",
- "pools get --pool-name=test_pool -o yaml",
+ "pools get test_pool",
+ "pools get test_pool -o yaml",
"pools update --pool=test_pool --slots=10",
"pools import tests/airflowctl_tests/fixtures/test_pools.json",
"pools export tests/airflowctl_tests/fixtures/pools_export.json
--output=json",
- "pools delete --pool=test_pool",
- "pools delete --pool=test_import_pool",
+ "pools delete test_pool",
+ "pools delete test_import_pool",
# Providers commands
"providers list",
# Variables commands
"variables create --key=test_key --value=test_value",
"variables list",
- "variables get --variable-key=test_key",
- "variables get --variable-key=test_key -o table",
+ "variables get test_key",
+ "variables get test_key -o table",
"variables update --key=test_key --value=updated_value",
"variables import tests/airflowctl_tests/fixtures/test_variables.json",
- "variables delete --variable-key=test_key",
- "variables delete --variable-key=test_import_var",
- "variables delete --variable-key=test_import_var_with_desc",
+ "variables delete test_key",
+ "variables delete test_import_var",
+ "variables delete test_import_var_with_desc",
# Plugins command
"plugins list",
"plugins list-import-errors",
diff --git
a/airflow-ctl-tests/tests/airflowctl_tests/test_config_sensitive_masking.py
b/airflow-ctl-tests/tests/airflowctl_tests/test_config_sensitive_masking.py
index 776629da3a2..a9e98d9365c 100644
--- a/airflow-ctl-tests/tests/airflowctl_tests/test_config_sensitive_masking.py
+++ b/airflow-ctl-tests/tests/airflowctl_tests/test_config_sensitive_masking.py
@@ -23,8 +23,8 @@ SENSITIVE_CONFIG_COMMANDS = [
# Test that config list shows masked sensitive values
"config list",
# Test that getting specific sensitive config values are masked
- "config get --section core --option fernet_key",
- "config get --section database --option sql_alchemy_conn",
+ "config get core fernet_key",
+ "config get database sql_alchemy_conn",
]
diff --git a/airflow-ctl/RELEASE_NOTES.rst b/airflow-ctl/RELEASE_NOTES.rst
index dc250d0ec04..0de56a413dc 100644
--- a/airflow-ctl/RELEASE_NOTES.rst
+++ b/airflow-ctl/RELEASE_NOTES.rst
@@ -15,6 +15,17 @@
specific language governing permissions and limitations
under the License.
+airflowctl unreleased
+---------------------
+
+Significant Changes
+^^^^^^^^^^^^^^^^^^^
+
+- Expose required primitive parameters of auto-generated commands as positional
+ arguments instead of ``--flag`` options. Optional parameters keep the
+ ``--flag`` form. Follows the dev-list lazy consensus on airflowctl parameter
+ style (see
`<https://lists.apache.org/thread/m1qvcvow3l17ytv40vhslh40wn3rntrm>`_).
+
airflowctl 0.1.4 (2026-04-18)
-----------------------------
diff --git a/airflow-ctl/src/airflowctl/ctl/cli_config.py
b/airflow-ctl/src/airflowctl/ctl/cli_config.py
index aa96304f501..1d3c30121b6 100755
--- a/airflow-ctl/src/airflowctl/ctl/cli_config.py
+++ b/airflow-ctl/src/airflowctl/ctl/cli_config.py
@@ -423,11 +423,19 @@ class CommandFactory:
args = []
return_annotation: str = ""
- for arg in node.args.args:
+ # In ``ast.arguments``, ``defaults`` aligns with the *tail* of
+ # ``args``. A parameter is required when its position from the
+ # left is *before* the first defaulted position. Equivalent to
+ # ``len(args) - len(defaults)``.
+ positional_args = [a for a in node.args.args if a.arg != "self"]
+ defaults_count = len(node.args.defaults)
+ required_count = len(positional_args) - defaults_count
+ required_param_names: set[str] = {a.arg for a in
positional_args[:required_count]}
+
+ for arg in positional_args:
arg_name = arg.arg
arg_type = ast.unparse(arg.annotation) if arg.annotation else
"Any"
- if arg_name != "self":
- args.append({arg_name: arg_type})
+ args.append({arg_name: arg_type})
if node.returns:
return_annotation = [
@@ -437,6 +445,7 @@ class CommandFactory:
return {
"name": func_name,
"parameters": args,
+ "required_param_names": required_param_names,
"return_type": return_annotation,
"parent": parent_node,
}
@@ -532,6 +541,26 @@ class CommandFactory:
action=arg_action,
)
+ @staticmethod
+ def _create_positional_arg(
+ parameter_key: str,
+ arg_type: type | Callable,
+ arg_help: str,
+ ) -> Arg:
+ """
+ Build a positional ``Arg`` for a required primitive parameter.
+
+ ``argparse`` rejects ``default`` and ``dest`` on positional arguments,
+ so this helper keeps both unset and uses the raw parameter name (with
+ underscores) as the flag so the parsed ``Namespace`` attribute lines up
+ with the operation method's signature.
+ """
+ return Arg(
+ flags=(parameter_key,),
+ type=arg_type,
+ help=arg_help,
+ )
+
def _create_arg_for_non_primitive_type(
self,
parameter_type: str,
@@ -577,20 +606,44 @@ class CommandFactory:
"""Create Arg from Operation Method checking for parameters and return
types."""
for operation in self.operations:
args = []
+ required_names: set[str] = operation.get("required_param_names")
or set()
for parameter in operation.get("parameters"):
for parameter_key, parameter_type in parameter.items():
if self._is_primitive_type(type_name=parameter_type):
base_parameter_type = parameter_type.replace(" |
None", "").strip()
is_bool = base_parameter_type == "bool"
- args.append(
- self._create_arg(
- arg_flags=("--" +
self._sanitize_arg_parameter_key(parameter_key),),
-
arg_type=self._python_type_from_string(parameter_type),
- arg_action=argparse.BooleanOptionalAction if
is_bool else None,
- arg_help=f"{parameter_key} for
{operation.get('name')} operation in {operation.get('parent').name}",
- arg_default=None,
- )
+ # Required, non-bool primitives are exposed as
positional
+ # arguments per the dev-list lazy consensus
+ #
(https://lists.apache.org/thread/m1qvcvow3l17ytv40vhslh40wn3rntrm).
+ # Bool stays --flag/--no-flag and ``parameter_type``
+ # ending in ``| None`` is treated as optional.
+ is_required_positional = (
+ parameter_key in required_names and not is_bool
and "| None" not in parameter_type
)
+ if is_required_positional:
+ args.append(
+ self._create_positional_arg(
+ parameter_key=parameter_key,
+
arg_type=self._python_type_from_string(parameter_type),
+ arg_help=(
+ f"{parameter_key} for
{operation.get('name')} "
+ f"operation in
{operation.get('parent').name}"
+ ),
+ )
+ )
+ else:
+ args.append(
+ self._create_arg(
+ arg_flags=("--" +
self._sanitize_arg_parameter_key(parameter_key),),
+
arg_type=self._python_type_from_string(parameter_type),
+ arg_action=argparse.BooleanOptionalAction
if is_bool else None,
+ arg_help=(
+ f"{parameter_key} for
{operation.get('name')} "
+ f"operation in
{operation.get('parent').name}"
+ ),
+ arg_default=None,
+ )
+ )
else:
args.extend(
self._create_arg_for_non_primitive_type(
diff --git a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py
b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py
index e0278cd7c53..945b5abb833 100644
--- a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py
+++ b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py
@@ -19,6 +19,7 @@ from __future__ import annotations
import argparse
from argparse import BooleanOptionalAction
+from pathlib import Path
from textwrap import dedent
import httpx
@@ -158,12 +159,10 @@ def test_args_list():
def test_args_get():
return [
(
- "--backfill-id",
+ "backfill_id",
{
"help": "backfill_id for get operation in BackfillsOperations",
- "default": None,
"type": str,
- "dest": None,
},
),
(
@@ -182,12 +181,10 @@ def test_args_get():
def test_args_delete():
return [
(
- "--backfill-id",
+ "backfill_id",
{
"help": "backfill_id for delete operation in
BackfillsOperations",
- "default": None,
"type": str,
- "dest": None,
},
),
(
@@ -204,26 +201,26 @@ def test_args_delete():
class TestCommandFactory:
@classmethod
- def _save_temp_operations_py(cls, temp_file: str, file_content) -> None:
+ def _save_temp_operations_py(cls, tmp_path: Path, file_content: str) ->
Path:
"""
Save a temporary operations.py file with a simple Command Class to
test the command factory.
- """
- with open(temp_file, "w") as f:
- f.write(dedent(file_content))
- def teardown_method(self):
- """
- Remove the temporary file after the test.
+ Writing inside a per-test ``tmp_path`` keeps the file isolated under
+ parallel execution (pytest-xdist), avoiding cross-worker overwrites of
+ a shared ``test_command.py`` in the cwd. Returns the full path.
"""
- try:
- import os
-
- os.remove("test_command.py")
- except FileNotFoundError:
- pass
+ temp_file = tmp_path / "test_command.py"
+ temp_file.write_text(dedent(file_content))
+ return temp_file
def test_command_factory(
- self, no_op_method, test_args_create, test_args_list, test_args_get,
test_args_delete
+ self,
+ tmp_path,
+ no_op_method,
+ test_args_create,
+ test_args_list,
+ test_args_get,
+ test_args_delete,
):
"""
Test the command factory.
@@ -231,9 +228,8 @@ class TestCommandFactory:
# Create temporary file with pytest and write simple Command
Class(check airflow-ctl/src/airflowctl/api/operations.py) to file
# to test the command factory
# Create a temporary file
- temp_file = "test_command.py"
- self._save_temp_operations_py(
- temp_file=temp_file,
+ temp_file = self._save_temp_operations_py(
+ tmp_path=tmp_path,
file_content="""
class NotAnOperation:
def test_method(self):
@@ -260,7 +256,7 @@ class TestCommandFactory:
""",
)
- command_factory = CommandFactory(file_path=temp_file)
+ command_factory = CommandFactory(file_path=str(temp_file))
generated_group_commands = command_factory.group_commands
for generated_group_command in generated_group_commands:
@@ -287,20 +283,19 @@ class TestCommandFactory:
for arg, test_arg in zip(sub_command.args, test_args_get):
assert arg.flags[0] == test_arg[0]
assert arg.kwargs["help"] == test_arg[1]["help"]
- assert arg.kwargs["default"] == test_arg[1]["default"]
+ assert arg.kwargs.get("default") ==
test_arg[1].get("default")
assert arg.kwargs["type"] == test_arg[1]["type"]
elif sub_command.name == "delete":
for arg, test_arg in zip(sub_command.args,
test_args_delete):
assert arg.flags[0] == test_arg[0]
assert arg.kwargs["help"] == test_arg[1]["help"]
- assert arg.kwargs["default"] == test_arg[1]["default"]
+ assert arg.kwargs.get("default") ==
test_arg[1].get("default")
assert arg.kwargs["type"] == test_arg[1]["type"]
- def test_command_factory_optional_bool_uses_boolean_optional_action(self):
+ def test_command_factory_optional_bool_uses_boolean_optional_action(self,
tmp_path):
"""Optional bool parameters should support --flag and --no-flag
forms."""
- temp_file = "test_command.py"
- self._save_temp_operations_py(
- temp_file=temp_file,
+ temp_file = self._save_temp_operations_py(
+ tmp_path=tmp_path,
file_content="""
class JobsOperations(BaseOperations):
def list(self, is_alive: bool | None = None) ->
JobCollectionResponse | ServerResponseError:
@@ -309,7 +304,7 @@ class TestCommandFactory:
""",
)
- command_factory = CommandFactory(file_path=temp_file)
+ command_factory = CommandFactory(file_path=str(temp_file))
generated_group_commands = command_factory.group_commands
jobs_list_args = []
@@ -326,6 +321,97 @@ class TestCommandFactory:
assert is_alive_arg.kwargs["default"] is None
assert is_alive_arg.kwargs["type"] is bool
+ def test_command_factory_required_primitive_param_is_positional(self,
tmp_path):
+ """Required primitive parameters (no default, not Optional) become
positional arguments.
+
+ Following the dev-list lazy consensus on ``airflowctl`` parameter style
+
(`<https://lists.apache.org/thread/m1qvcvow3l17ytv40vhslh40wn3rntrm>`_),
+ required parameters of auto-generated commands should be positional and
+ optional parameters keep the ``--flag`` form.
+ """
+ temp_file = self._save_temp_operations_py(
+ tmp_path=tmp_path,
+ file_content="""
+ class WidgetsOperations(BaseOperations):
+ def get(self, widget_id: str) -> WidgetResponse |
ServerResponseError:
+ self.response = self.client.get(f"widgets/{widget_id}")
+ return
WidgetResponse.model_validate_json(self.response.content)
+ def delete(self, widget_id: str) -> ServerResponseError |
None:
+ self.response =
self.client.delete(f"widgets/{widget_id}")
+ return None
+ def update_version(
+ self, widget_id: str, version: int, note: str | None =
None
+ ) -> WidgetResponse | ServerResponseError:
+ self.response = self.client.patch(
+ f"widgets/{widget_id}/version/{version}",
+ json={"note": note},
+ )
+ return
WidgetResponse.model_validate_json(self.response.content)
+ """,
+ )
+
+ command_factory = CommandFactory(file_path=str(temp_file))
+ sub_commands = {}
+ for generated_group_command in command_factory.group_commands:
+ if generated_group_command.name != "widgets":
+ continue
+ for sub_command in generated_group_command.subcommands:
+ sub_commands[sub_command.name] = sub_command
+
+ # get: single required str -> positional
+ get_args = list(sub_commands["get"].args)
+ widget_id_arg = next(arg for arg in get_args if arg.flags[0] in
("widget_id", "--widget-id"))
+ assert widget_id_arg.flags == ("widget_id",)
+ assert widget_id_arg.kwargs["type"] is str
+ assert "default" not in widget_id_arg.kwargs
+ assert "action" not in widget_id_arg.kwargs
+
+ # delete: same shape
+ delete_args = list(sub_commands["delete"].args)
+ delete_widget_id_arg = next(
+ arg for arg in delete_args if arg.flags[0] in ("widget_id",
"--widget-id")
+ )
+ assert delete_widget_id_arg.flags == ("widget_id",)
+
+ # update-version: two required (str + int) positional, ``note`` keeps
--flag form
+ update_args = list(sub_commands["update-version"].args)
+ update_widget_id_arg = next(
+ arg for arg in update_args if arg.flags[0] in ("widget_id",
"--widget-id")
+ )
+ version_arg = next(arg for arg in update_args if arg.flags[0] in
("version", "--version"))
+ note_arg = next(arg for arg in update_args if arg.flags[0] in ("note",
"--note"))
+ assert update_widget_id_arg.flags == ("widget_id",)
+ assert version_arg.flags == ("version",)
+ assert version_arg.kwargs["type"] is int
+ assert note_arg.flags == ("--note",)
+ assert note_arg.kwargs["default"] is None
+
+ def test_command_factory_primitive_param_with_default_keeps_flag(self,
tmp_path):
+ """Primitive parameters with a default value keep the ``--flag``
form."""
+ temp_file = self._save_temp_operations_py(
+ tmp_path=tmp_path,
+ file_content="""
+ class WidgetsOperations(BaseOperations):
+ def list(self, limit: int = 100) ->
WidgetCollectionResponse | ServerResponseError:
+ self.response = self.client.get("widgets",
params={"limit": limit})
+ return
WidgetCollectionResponse.model_validate_json(self.response.content)
+ """,
+ )
+
+ command_factory = CommandFactory(file_path=str(temp_file))
+ list_args = []
+ for generated_group_command in command_factory.group_commands:
+ if generated_group_command.name != "widgets":
+ continue
+ for sub_command in generated_group_command.subcommands:
+ if sub_command.name == "list":
+ list_args = list(sub_command.args)
+ break
+
+ limit_arg = next(arg for arg in list_args if arg.flags[0] in ("limit",
"--limit"))
+ assert limit_arg.flags == ("--limit",)
+ assert limit_arg.kwargs["type"] is int
+
class TestCliConfigMethods:
@pytest.mark.parametrize(
