Make required params positional for auto-generated airflowctl commands

[shivaam]

Required non-boolean parameters in auto-generated CLI commands are now positional instead of flag-style, improving UX consistency with manually defined commands like dags pause.

Before:

airflowctl connections create --connection-id="test" --conn-type="mysql" --password=secret
airflowctl dags get --dag-id=example_bash_operator

After:

airflowctl connections create test mysql --password=secret
airflowctl dags get example_bash_operator

Changes

• Track which operation parameters have default values during AST parsing
• For primitive params, use the default info to decide positional vs flag
• For Pydantic model fields, use is_required() to decide positional vs flag
• Use _UNSET for arg_dest default to avoid argparse crash on positional args
• Skip the has_default metadata key at runtime when mapping CLI args to method params
• Updated unit tests and integration test commands to match new positional syntax
• Added 2 new tests for positional arg behavior and default detection
• Added newsfragment for the breaking change

Notes

• _create_arg stays a dumb pass-through builder — all positional/flag logic is in the callers via one-liner conditionals
• Booleans are never positional (they need --flag/--no-flag)
• No special-casing for specific operations
• Breaking change: existing --flag=value syntax for required params will no longer work. Since airflowctl is new and not yet stable, this should be acceptable.

Open question

Posted a clarifying question on the issue about whether all required params should be positional (e.g., xcom add has 5 positional args), or if we should limit it.

closes: #60142

---

Was generative AI tooling used to co-author this PR?

• Yes — Claude Code (Claude Opus 4.6)

Generated-by: Claude Code (Claude Opus 4.6) following the guidelines

[bugraoz93]

Thanks for the PR! I addressed your question. Could you please amend it to be both optional and positional, live side by side?
This way, we won't break anything while working, but rather add Airflow CLI positional arguments to improve parity and make migrations easier.

[bugraoz93]

This news fragment is for Airflow Core and not related to anything within airflowctl flow.
Since we are still in 0.x, significant changes are also expected at a certain level.
We are logging these things in the release notes

https://airflow.apache.org/docs/apache-airflow-ctl/stable/release_notes.html#significant-changes

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates auto-generated airflowctl commands so required non-boolean parameters are positional (UX aligned with manually-defined commands), and adjusts parsing/tests to support the breaking CLI syntax change.

Changes:

• Track whether operation parameters have defaults during AST parsing and use it to decide positional vs flag args for primitive params.
• Use Pydantic model_fields[].is_required() to decide positional vs flag args for datamodel fields (booleans remain flags).
• Update unit/integration tests and add a newsfragment documenting the breaking change.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py	Updates assertions for positional args; adds tests for positional behavior and default detection.
airflow-ctl/src/airflowctl/ctl/cli_config.py	Adds has_default metadata via AST parsing; generates positional args for required params; skips metadata during arg mapping and invocation.
airflow-ctl-tests/tests/airflowctl_tests/test_config_sensitive_masking.py	Updates integration test commands to new positional syntax.
airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py	Updates end-to-end command list to match new positional syntax across many commands.
airflow-core/newsfragments/64812.significant.rst	Documents the breaking CLI change for auto-generated commands.

[Copilot]

The parameters entries are now shaped as a dict with a dynamic key for the param name plus a reserved has_default key. This forces downstream code to iterate parameter.items() and special-case parameter_key == \"has_default\", which is brittle and harder to extend (and also makes collisions with a real param named has_default difficult to reason about). Consider switching to a stable structure like { \"name\": ..., \"type\": ..., \"has_default\": ... } for each parameter so consumers can access fields directly without item-iteration + skips.

Suggested change

	args.append({arg_name: arg_type, "has_default": has_default})
	args.append(
	{"name": arg_name, "type": arg_type, "has_default": has_default}
	)

[Copilot]

Related to the parameter-shape issue: the for parameter_key, parameter_type in parameter.items() loop plus if parameter_key == \"has_default\": continue is an avoidable control-flow hack that spreads across multiple call sites (also in _get_func). If you keep the current shape, a safer alternative is to store metadata under a dedicated nested key (e.g. {\"name\": ..., \"type\": ..., \"meta\": {\"has_default\": ...}}) or move metadata out of the per-parameter dict entirely so this loop can be simplified and cannot accidentally skip real parameters.

[Copilot]

This test writes a fixed filename (test_has_default.py) into the working directory, which can conflict under parallel test runs or when the CWD is not writable. Use pytest’s tmp_path/tmp_path_factory (or tempfile.NamedTemporaryFile) to create an isolated temporary file path, and open it with an explicit encoding (e.g. encoding=\"utf-8\") to match the production file-read behavior.

Suggested change

	deftest_has_default_detection_in_ast_parsing(self):
	"""Test that AST parsing correctly detects which params have defaults."""
	fromtextwrapimportdedent
	temp_file="test_has_default.py"
	withopen(temp_file, "w") asf:
	f.write(
	dedent("""
	classTestOperations(BaseOperations):
	defget(self, required_id: str) ->str|ServerResponseError:
	pass
	defsearch(self, query: str, limit: int=10, offset: int=0) ->str|ServerResponseError:
	pass
	""")
	)
	try:
	command_factory=CommandFactory(file_path=temp_file)
	foropincommand_factory.operations:
	ifop["name"] =="get":
	param=op["parameters"][0]
	assertparam.get("has_default") isFalse
	elifop["name"] =="search":
	query_param=op["parameters"][0]
	assertquery_param.get("has_default") isFalse
	limit_param=op["parameters"][1]
	assertlimit_param.get("has_default") isTrue
	offset_param=op["parameters"][2]
	assertoffset_param.get("has_default") isTrue
	finally:
	importos
	os.remove(temp_file)
	deftest_has_default_detection_in_ast_parsing(self, tmp_path):
	"""Test that AST parsing correctly detects which params have defaults."""
	fromtextwrapimportdedent
	temp_file=tmp_path/"test_has_default.py"
	temp_file.write_text(
	dedent("""
	classTestOperations(BaseOperations):
	defget(self, required_id: str) ->str|ServerResponseError:
	pass
	defsearch(self, query: str, limit: int=10, offset: int=0) ->str|ServerResponseError:
	pass
	"""),
	encoding="utf-8",
	)
	command_factory=CommandFactory(file_path=str(temp_file))
	foropincommand_factory.operations:
	ifop["name"] =="get":
	param=op["parameters"][0]
	assertparam.get("has_default") isFalse
	elifop["name"] =="search":
	query_param=op["parameters"][0]
	assertquery_param.get("has_default") isFalse
	limit_param=op["parameters"][1]
	assertlimit_param.get("has_default") isTrue
	offset_param=op["parameters"][2]
	assertoffset_param.get("has_default") isTrue

[bugraoz93]

Moving to draft to manage release better, please make it ready again when you think ready for maintainers review

[shivaam]

Moving to draft to manage release better, please make it ready again when you think ready for maintainers review
@bugraoz93!

Sorry I replied on the issue instead of the PR. argparse doesn't natively support this. Below I have listed some options.

---

I looked into supporting both positional and --flag styles for required params. Unfortunately argparse doesn't natively support this — a parameter is either positional (dag_id) or a flag (--dag-id), never both.

Why it's not straightforward:

The closest workaround is registering all params as --flags, adding a nargs='*' catch-all for bare values, and using parse_intermixed_args to map positionals into unfilled flag slots after parsing. More about this at the bottom of this comment. But this means we lose argparse's built-in validation, type checking, help formatting, and required=True for those params — we'd essentially reimplement positional argument handling ourselves on top of argparse.

Options:

1. Keep it positional for required params (current PR) — the simplest, most conventional choice. Required params are positional, optional params are flags. All argparse features work, clean help output.

2. Two-pass parsing — maintain two parsers per command: one with required params as positional, one with them as --flags required=True. Try positional first, fall back to flags. Users can use either style, just not mixed in the same invocation:

   # both work:
   airflowctl xcom add my_dag my_task my_key
   airflowctl xcom add --dag-id=my_dag --task-id=my_task --key=my_key
   # but not mixed:
   airflowctl xcom add my_dag --task-id=my_task my_key # would fail

   No argparse hacks needed (both parsers are standard), but doubles the parser setup and mixed usage isn't supported.

3. Cap positional count — first 1-2 required params (like dag_id) are positional, rest stay as --flag required=True. Common CLI pattern (like git checkout <branch> --force). No hacks, all argparse features work.

I'd recommend Option 1 for simplicity. Happy to go with whichever you prefer!

---

Details on the parse_intermixed_args workaround (and why I didn't recommend it)

The idea is to register all required params as --flags only (no argparse positionals), add a single nargs='*' argument to collect any bare values, then map those bare values into whichever flag slots the user didn't fill:

PARAM_ORDER= ['dag_id', 'task_id', 'key']
parser.add_argument('--dag-id', dest='dag_id', default=None)
parser.add_argument('--task-id', dest='task_id', default=None)
parser.add_argument('--key', default=None)
parser.add_argument('positionals', nargs='*')
args=parser.parse_intermixed_args(argv)
# map bare values into unfilled flag slots, left to rightunfilled= [dfordinPARAM_ORDERifgetattr(args, d) isNone]
fori, valinenumerate(args.positionals):
setattr(args, unfilled[i], val)

parse_intermixed_args (Python 3.7+) allows flags and bare values to be freely interspersed, so all of these would work:

airflowctl xcom add d t k # all positional
airflowctl xcom add --dag-id=d --task-id=t --key=k # all flags
airflowctl xcom add d --task-id=t k # mixed
airflowctl xcom add --task-id=t my_dag my_key # gap-filling

The problem is what we lose:

• Validation — can't use required=True on flags (argparse would reject positional-only usage), so we validate manually after gap-filling
• Type checking — type=int, choices=[...] etc. don't apply to the nargs='*' catch-all, so we type-check manually
• Help output — shows [positionals ...] instead of named params like DAG_ID TASK_ID, and no indication which flags are required vs optional
• argparse.REMAINDER and mutually exclusive groups with positionals — unsupported with parse_intermixed_args

This effectively means using argparse for flag parsing only and reimplementing positional handling on top of it.

[bugraoz93]

@shivaam, this will make things a bit complicated. The two-pass parsing is still complex. I would say before we finish adding the command to the parser, we can reprocess the args, maybe even in get_parser, which can also be responsible for parsing the command. We may need to update the building part a bit but it should be possible with reprocessing

Even though we are still around 0.x.x, still thinking about how we could implement this in the best position. As it will be a user-facing change, we need to create a devlist discussion and ask to only keep positional for required or maintain both.

My idea is adding both can save us from the entire discussion flow and we could be backwards compatible, even though this is not trivial with argparase in Python.

[bugraoz93]

Something like this
#65261

[shivaam]

Closing in favor of #65261 which takes a better approach — supporting both positional and flag styles simultaneously via argv pre-processing, so there's no breaking change for existing users. Thanks @bugraoz93 for the guidance!

[shivaam]

Something like this #65261

I was just thinking about this. Supporting both positional and flag styles for the same param isn't something most CLI libraries or popular tools do. We're adding maintenance burden, constraining future changes as all CLI libraries might not suppor this feature, and expanding the testing surface forever. Since airflowctl is still pre-1.0, feels like this is the natural window for a clean break rather than committing to maintaining a non-standard approach indefinitely.

Since the layer we are adding is light, either way works. Just wanted to share my thoughts.

[bugraoz93]

Thanks @shivaam for your patience and joining the discussion! We have some procedures to follow as a community. :) Reopened yours, feel free to rebase and if ready we can ship

[bugraoz93]

https://lists.apache.org/thread/m1qvcvow3l17ytv40vhslh40wn3rntrm

[shivaam]

Sounds good. I will rebase the project.

[shivaam]

Before this PR, airflowctl backfill list (no args) returned [] and exited 0, but that was a silent bug — the CLI passed dag_id=None, which the server coerced to an empty filter that matched no records. The command was never actually returning backfills.

[bugraoz93]

Looks good! Thanks for your patience and work @shivaam!

[bugraoz93]

Added the full test label to see the integration tests green. We can merge after seeing it green :)

[bugraoz93]

CI looks unrelated, ctl test looks good

[bugraoz93]

@shivaam Could you please rebase and resolve conflicts, we can merge afterwards

[shivaam]

@bugraoz93 Seems like there is another PR that already solved this problem PR #66768 — "airflowctl: make required CLI params positional, keep optional as --flag" by 1fanwang
I dont think we need this anymore.

[bugraoz93]

Sorry @shivaam! Totally missed that one. I will try to be faster next time 😅 It also has some release notes changes need manually handled while backporting and should be cleaned while generating

[shivaam]

Required non-boolean parameters in auto-generated CLI commands are now positional instead of flag-style, improving UX consistency with manually defined commands like dags pause.

Before:

airflowctl connections create --connection-id="test" --conn-type="mysql" --password=secret
airflowctl dags get --dag-id=example_bash_operator

After:

airflowctl connections create test mysql --password=secret
airflowctl dags get example_bash_operator

Changes

• Track which operation parameters have default values during AST parsing
• For primitive params, use the default info to decide positional vs flag
• For Pydantic model fields, use is_required() to decide positional vs flag
• Use _UNSET for arg_dest default to avoid argparse crash on positional args
• Skip the has_default metadata key at runtime when mapping CLI args to method params
• Updated unit tests and integration test commands to match new positional syntax
• Added 2 new tests for positional arg behavior and default detection
• Added newsfragment for the breaking change

Notes

• _create_arg stays a dumb pass-through builder — all positional/flag logic is in the callers via one-liner conditionals
• Booleans are never positional (they need --flag/--no-flag)
• No special-casing for specific operations
• Breaking change: existing --flag=value syntax for required params will no longer work. Since airflowctl is new and not yet stable, this should be acceptable.

Open question

Posted a clarifying question on the issue about whether all required params should be positional (e.g., xcom add has 5 positional args), or if we should limit it.

closes: #60142

---

Was generative AI tooling used to co-author this PR?

• Yes — Claude Code (Claude Opus 4.6)

Generated-by: Claude Code (Claude Opus 4.6) following the guidelines

[bugraoz93]

Thanks for the PR! I addressed your question. Could you please amend it to be both optional and positional, live side by side?
This way, we won't break anything while working, but rather add Airflow CLI positional arguments to improve parity and make migrations easier.

[bugraoz93]

This news fragment is for Airflow Core and not related to anything within airflowctl flow.
Since we are still in 0.x, significant changes are also expected at a certain level.
We are logging these things in the release notes

https://airflow.apache.org/docs/apache-airflow-ctl/stable/release_notes.html#significant-changes

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates auto-generated airflowctl commands so required non-boolean parameters are positional (UX aligned with manually-defined commands), and adjusts parsing/tests to support the breaking CLI syntax change.

Changes:

• Track whether operation parameters have defaults during AST parsing and use it to decide positional vs flag args for primitive params.
• Use Pydantic model_fields[].is_required() to decide positional vs flag args for datamodel fields (booleans remain flags).
• Update unit/integration tests and add a newsfragment documenting the breaking change.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py	Updates assertions for positional args; adds tests for positional behavior and default detection.
airflow-ctl/src/airflowctl/ctl/cli_config.py	Adds has_default metadata via AST parsing; generates positional args for required params; skips metadata during arg mapping and invocation.
airflow-ctl-tests/tests/airflowctl_tests/test_config_sensitive_masking.py	Updates integration test commands to new positional syntax.
airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py	Updates end-to-end command list to match new positional syntax across many commands.
airflow-core/newsfragments/64812.significant.rst	Documents the breaking CLI change for auto-generated commands.

[Copilot]

The parameters entries are now shaped as a dict with a dynamic key for the param name plus a reserved has_default key. This forces downstream code to iterate parameter.items() and special-case parameter_key == \"has_default\", which is brittle and harder to extend (and also makes collisions with a real param named has_default difficult to reason about). Consider switching to a stable structure like { \"name\": ..., \"type\": ..., \"has_default\": ... } for each parameter so consumers can access fields directly without item-iteration + skips.

Suggested change

	args.append({arg_name: arg_type, "has_default": has_default})
	args.append(
	{"name": arg_name, "type": arg_type, "has_default": has_default}
	)

[Copilot]

Related to the parameter-shape issue: the for parameter_key, parameter_type in parameter.items() loop plus if parameter_key == \"has_default\": continue is an avoidable control-flow hack that spreads across multiple call sites (also in _get_func). If you keep the current shape, a safer alternative is to store metadata under a dedicated nested key (e.g. {\"name\": ..., \"type\": ..., \"meta\": {\"has_default\": ...}}) or move metadata out of the per-parameter dict entirely so this loop can be simplified and cannot accidentally skip real parameters.

[Copilot]

This test writes a fixed filename (test_has_default.py) into the working directory, which can conflict under parallel test runs or when the CWD is not writable. Use pytest’s tmp_path/tmp_path_factory (or tempfile.NamedTemporaryFile) to create an isolated temporary file path, and open it with an explicit encoding (e.g. encoding=\"utf-8\") to match the production file-read behavior.

Suggested change

	deftest_has_default_detection_in_ast_parsing(self):
	"""Test that AST parsing correctly detects which params have defaults."""
	fromtextwrapimportdedent
	temp_file="test_has_default.py"
	withopen(temp_file, "w") asf:
	f.write(
	dedent("""
	classTestOperations(BaseOperations):
	defget(self, required_id: str) ->str|ServerResponseError:
	pass
	defsearch(self, query: str, limit: int=10, offset: int=0) ->str|ServerResponseError:
	pass
	""")
	)
	try:
	command_factory=CommandFactory(file_path=temp_file)
	foropincommand_factory.operations:
	ifop["name"] =="get":
	param=op["parameters"][0]
	assertparam.get("has_default") isFalse
	elifop["name"] =="search":
	query_param=op["parameters"][0]
	assertquery_param.get("has_default") isFalse
	limit_param=op["parameters"][1]
	assertlimit_param.get("has_default") isTrue
	offset_param=op["parameters"][2]
	assertoffset_param.get("has_default") isTrue
	finally:
	importos
	os.remove(temp_file)
	deftest_has_default_detection_in_ast_parsing(self, tmp_path):
	"""Test that AST parsing correctly detects which params have defaults."""
	fromtextwrapimportdedent
	temp_file=tmp_path/"test_has_default.py"
	temp_file.write_text(
	dedent("""
	classTestOperations(BaseOperations):
	defget(self, required_id: str) ->str|ServerResponseError:
	pass
	defsearch(self, query: str, limit: int=10, offset: int=0) ->str|ServerResponseError:
	pass
	"""),
	encoding="utf-8",
	)
	command_factory=CommandFactory(file_path=str(temp_file))
	foropincommand_factory.operations:
	ifop["name"] =="get":
	param=op["parameters"][0]
	assertparam.get("has_default") isFalse
	elifop["name"] =="search":
	query_param=op["parameters"][0]
	assertquery_param.get("has_default") isFalse
	limit_param=op["parameters"][1]
	assertlimit_param.get("has_default") isTrue
	offset_param=op["parameters"][2]
	assertoffset_param.get("has_default") isTrue

[bugraoz93]

Moving to draft to manage release better, please make it ready again when you think ready for maintainers review

[shivaam]

Moving to draft to manage release better, please make it ready again when you think ready for maintainers review
@bugraoz93!

Sorry I replied on the issue instead of the PR. argparse doesn't natively support this. Below I have listed some options.

---

I looked into supporting both positional and --flag styles for required params. Unfortunately argparse doesn't natively support this — a parameter is either positional (dag_id) or a flag (--dag-id), never both.

Why it's not straightforward:

The closest workaround is registering all params as --flags, adding a nargs='*' catch-all for bare values, and using parse_intermixed_args to map positionals into unfilled flag slots after parsing. More about this at the bottom of this comment. But this means we lose argparse's built-in validation, type checking, help formatting, and required=True for those params — we'd essentially reimplement positional argument handling ourselves on top of argparse.

Options:

1. Keep it positional for required params (current PR) — the simplest, most conventional choice. Required params are positional, optional params are flags. All argparse features work, clean help output.

2. Two-pass parsing — maintain two parsers per command: one with required params as positional, one with them as --flags required=True. Try positional first, fall back to flags. Users can use either style, just not mixed in the same invocation:

   # both work:
   airflowctl xcom add my_dag my_task my_key
   airflowctl xcom add --dag-id=my_dag --task-id=my_task --key=my_key
   # but not mixed:
   airflowctl xcom add my_dag --task-id=my_task my_key # would fail

   No argparse hacks needed (both parsers are standard), but doubles the parser setup and mixed usage isn't supported.

3. Cap positional count — first 1-2 required params (like dag_id) are positional, rest stay as --flag required=True. Common CLI pattern (like git checkout <branch> --force). No hacks, all argparse features work.

I'd recommend Option 1 for simplicity. Happy to go with whichever you prefer!

---

Details on the parse_intermixed_args workaround (and why I didn't recommend it)

The idea is to register all required params as --flags only (no argparse positionals), add a single nargs='*' argument to collect any bare values, then map those bare values into whichever flag slots the user didn't fill:

PARAM_ORDER= ['dag_id', 'task_id', 'key']
parser.add_argument('--dag-id', dest='dag_id', default=None)
parser.add_argument('--task-id', dest='task_id', default=None)
parser.add_argument('--key', default=None)
parser.add_argument('positionals', nargs='*')
args=parser.parse_intermixed_args(argv)
# map bare values into unfilled flag slots, left to rightunfilled= [dfordinPARAM_ORDERifgetattr(args, d) isNone]
fori, valinenumerate(args.positionals):
setattr(args, unfilled[i], val)

parse_intermixed_args (Python 3.7+) allows flags and bare values to be freely interspersed, so all of these would work:

airflowctl xcom add d t k # all positional
airflowctl xcom add --dag-id=d --task-id=t --key=k # all flags
airflowctl xcom add d --task-id=t k # mixed
airflowctl xcom add --task-id=t my_dag my_key # gap-filling

The problem is what we lose:

• Validation — can't use required=True on flags (argparse would reject positional-only usage), so we validate manually after gap-filling
• Type checking — type=int, choices=[...] etc. don't apply to the nargs='*' catch-all, so we type-check manually
• Help output — shows [positionals ...] instead of named params like DAG_ID TASK_ID, and no indication which flags are required vs optional
• argparse.REMAINDER and mutually exclusive groups with positionals — unsupported with parse_intermixed_args

This effectively means using argparse for flag parsing only and reimplementing positional handling on top of it.

[bugraoz93]

@shivaam, this will make things a bit complicated. The two-pass parsing is still complex. I would say before we finish adding the command to the parser, we can reprocess the args, maybe even in get_parser, which can also be responsible for parsing the command. We may need to update the building part a bit but it should be possible with reprocessing

Even though we are still around 0.x.x, still thinking about how we could implement this in the best position. As it will be a user-facing change, we need to create a devlist discussion and ask to only keep positional for required or maintain both.

My idea is adding both can save us from the entire discussion flow and we could be backwards compatible, even though this is not trivial with argparase in Python.

[bugraoz93]

Something like this
#65261

[shivaam]

Closing in favor of #65261 which takes a better approach — supporting both positional and flag styles simultaneously via argv pre-processing, so there's no breaking change for existing users. Thanks @bugraoz93 for the guidance!

[shivaam]

Something like this #65261

I was just thinking about this. Supporting both positional and flag styles for the same param isn't something most CLI libraries or popular tools do. We're adding maintenance burden, constraining future changes as all CLI libraries might not suppor this feature, and expanding the testing surface forever. Since airflowctl is still pre-1.0, feels like this is the natural window for a clean break rather than committing to maintaining a non-standard approach indefinitely.

Since the layer we are adding is light, either way works. Just wanted to share my thoughts.

[bugraoz93]

Thanks @shivaam for your patience and joining the discussion! We have some procedures to follow as a community. :) Reopened yours, feel free to rebase and if ready we can ship

[bugraoz93]

https://lists.apache.org/thread/m1qvcvow3l17ytv40vhslh40wn3rntrm

[shivaam]

Sounds good. I will rebase the project.

[shivaam]

Before this PR, airflowctl backfill list (no args) returned [] and exited 0, but that was a silent bug — the CLI passed dag_id=None, which the server coerced to an empty filter that matched no records. The command was never actually returning backfills.

[bugraoz93]

Looks good! Thanks for your patience and work @shivaam!

[bugraoz93]

Added the full test label to see the integration tests green. We can merge after seeing it green :)

[bugraoz93]

CI looks unrelated, ctl test looks good

[bugraoz93]

@shivaam Could you please rebase and resolve conflicts, we can merge afterwards

[shivaam]

@bugraoz93 Seems like there is another PR that already solved this problem PR #66768 — "airflowctl: make required CLI params positional, keep optional as --flag" by 1fanwang
I dont think we need this anymore.

[bugraoz93]

Sorry @shivaam! Totally missed that one. I will try to be faster next time 😅 It also has some release notes changes need manually handled while backporting and should be cleaned while generating

[shivaam]

Required non-boolean parameters in auto-generated CLI commands are now positional instead of flag-style, improving UX consistency with manually defined commands like dags pause.

Before:

airflowctl connections create --connection-id="test" --conn-type="mysql" --password=secret
airflowctl dags get --dag-id=example_bash_operator

After:

airflowctl connections create test mysql --password=secret
airflowctl dags get example_bash_operator

Changes

• Track which operation parameters have default values during AST parsing
• For primitive params, use the default info to decide positional vs flag
• For Pydantic model fields, use is_required() to decide positional vs flag
• Use _UNSET for arg_dest default to avoid argparse crash on positional args
• Skip the has_default metadata key at runtime when mapping CLI args to method params
• Updated unit tests and integration test commands to match new positional syntax
• Added 2 new tests for positional arg behavior and default detection
• Added newsfragment for the breaking change

Notes

• _create_arg stays a dumb pass-through builder — all positional/flag logic is in the callers via one-liner conditionals
• Booleans are never positional (they need --flag/--no-flag)
• No special-casing for specific operations
• Breaking change: existing --flag=value syntax for required params will no longer work. Since airflowctl is new and not yet stable, this should be acceptable.

Open question

Posted a clarifying question on the issue about whether all required params should be positional (e.g., xcom add has 5 positional args), or if we should limit it.

closes: #60142

---

Was generative AI tooling used to co-author this PR?

• Yes — Claude Code (Claude Opus 4.6)

Generated-by: Claude Code (Claude Opus 4.6) following the guidelines

[bugraoz93]

Thanks for the PR! I addressed your question. Could you please amend it to be both optional and positional, live side by side?
This way, we won't break anything while working, but rather add Airflow CLI positional arguments to improve parity and make migrations easier.

[bugraoz93]

This news fragment is for Airflow Core and not related to anything within airflowctl flow.
Since we are still in 0.x, significant changes are also expected at a certain level.
We are logging these things in the release notes

https://airflow.apache.org/docs/apache-airflow-ctl/stable/release_notes.html#significant-changes

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates auto-generated airflowctl commands so required non-boolean parameters are positional (UX aligned with manually-defined commands), and adjusts parsing/tests to support the breaking CLI syntax change.

Changes:

• Track whether operation parameters have defaults during AST parsing and use it to decide positional vs flag args for primitive params.
• Use Pydantic model_fields[].is_required() to decide positional vs flag args for datamodel fields (booleans remain flags).
• Update unit/integration tests and add a newsfragment documenting the breaking change.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py	Updates assertions for positional args; adds tests for positional behavior and default detection.
airflow-ctl/src/airflowctl/ctl/cli_config.py	Adds has_default metadata via AST parsing; generates positional args for required params; skips metadata during arg mapping and invocation.
airflow-ctl-tests/tests/airflowctl_tests/test_config_sensitive_masking.py	Updates integration test commands to new positional syntax.
airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py	Updates end-to-end command list to match new positional syntax across many commands.
airflow-core/newsfragments/64812.significant.rst	Documents the breaking CLI change for auto-generated commands.

[Copilot]

The parameters entries are now shaped as a dict with a dynamic key for the param name plus a reserved has_default key. This forces downstream code to iterate parameter.items() and special-case parameter_key == \"has_default\", which is brittle and harder to extend (and also makes collisions with a real param named has_default difficult to reason about). Consider switching to a stable structure like { \"name\": ..., \"type\": ..., \"has_default\": ... } for each parameter so consumers can access fields directly without item-iteration + skips.

Suggested change

	args.append({arg_name: arg_type, "has_default": has_default})
	args.append(
	{"name": arg_name, "type": arg_type, "has_default": has_default}
	)

[Copilot]

Related to the parameter-shape issue: the for parameter_key, parameter_type in parameter.items() loop plus if parameter_key == \"has_default\": continue is an avoidable control-flow hack that spreads across multiple call sites (also in _get_func). If you keep the current shape, a safer alternative is to store metadata under a dedicated nested key (e.g. {\"name\": ..., \"type\": ..., \"meta\": {\"has_default\": ...}}) or move metadata out of the per-parameter dict entirely so this loop can be simplified and cannot accidentally skip real parameters.

[Copilot]

This test writes a fixed filename (test_has_default.py) into the working directory, which can conflict under parallel test runs or when the CWD is not writable. Use pytest’s tmp_path/tmp_path_factory (or tempfile.NamedTemporaryFile) to create an isolated temporary file path, and open it with an explicit encoding (e.g. encoding=\"utf-8\") to match the production file-read behavior.

Suggested change

	deftest_has_default_detection_in_ast_parsing(self):
	"""Test that AST parsing correctly detects which params have defaults."""
	fromtextwrapimportdedent
	temp_file="test_has_default.py"
	withopen(temp_file, "w") asf:
	f.write(
	dedent("""
	classTestOperations(BaseOperations):
	defget(self, required_id: str) ->str|ServerResponseError:
	pass
	defsearch(self, query: str, limit: int=10, offset: int=0) ->str|ServerResponseError:
	pass
	""")
	)
	try:
	command_factory=CommandFactory(file_path=temp_file)
	foropincommand_factory.operations:
	ifop["name"] =="get":
	param=op["parameters"][0]
	assertparam.get("has_default") isFalse
	elifop["name"] =="search":
	query_param=op["parameters"][0]
	assertquery_param.get("has_default") isFalse
	limit_param=op["parameters"][1]
	assertlimit_param.get("has_default") isTrue
	offset_param=op["parameters"][2]
	assertoffset_param.get("has_default") isTrue
	finally:
	importos
	os.remove(temp_file)
	deftest_has_default_detection_in_ast_parsing(self, tmp_path):
	"""Test that AST parsing correctly detects which params have defaults."""
	fromtextwrapimportdedent
	temp_file=tmp_path/"test_has_default.py"
	temp_file.write_text(
	dedent("""
	classTestOperations(BaseOperations):
	defget(self, required_id: str) ->str|ServerResponseError:
	pass
	defsearch(self, query: str, limit: int=10, offset: int=0) ->str|ServerResponseError:
	pass
	"""),
	encoding="utf-8",
	)
	command_factory=CommandFactory(file_path=str(temp_file))
	foropincommand_factory.operations:
	ifop["name"] =="get":
	param=op["parameters"][0]
	assertparam.get("has_default") isFalse
	elifop["name"] =="search":
	query_param=op["parameters"][0]
	assertquery_param.get("has_default") isFalse
	limit_param=op["parameters"][1]
	assertlimit_param.get("has_default") isTrue
	offset_param=op["parameters"][2]
	assertoffset_param.get("has_default") isTrue

[bugraoz93]

Moving to draft to manage release better, please make it ready again when you think ready for maintainers review

[shivaam]

Moving to draft to manage release better, please make it ready again when you think ready for maintainers review
@bugraoz93!

Sorry I replied on the issue instead of the PR. argparse doesn't natively support this. Below I have listed some options.

---

I looked into supporting both positional and --flag styles for required params. Unfortunately argparse doesn't natively support this — a parameter is either positional (dag_id) or a flag (--dag-id), never both.

Why it's not straightforward:

The closest workaround is registering all params as --flags, adding a nargs='*' catch-all for bare values, and using parse_intermixed_args to map positionals into unfilled flag slots after parsing. More about this at the bottom of this comment. But this means we lose argparse's built-in validation, type checking, help formatting, and required=True for those params — we'd essentially reimplement positional argument handling ourselves on top of argparse.

Options:

1. Keep it positional for required params (current PR) — the simplest, most conventional choice. Required params are positional, optional params are flags. All argparse features work, clean help output.

2. Two-pass parsing — maintain two parsers per command: one with required params as positional, one with them as --flags required=True. Try positional first, fall back to flags. Users can use either style, just not mixed in the same invocation:

   # both work:
   airflowctl xcom add my_dag my_task my_key
   airflowctl xcom add --dag-id=my_dag --task-id=my_task --key=my_key
   # but not mixed:
   airflowctl xcom add my_dag --task-id=my_task my_key # would fail

   No argparse hacks needed (both parsers are standard), but doubles the parser setup and mixed usage isn't supported.

3. Cap positional count — first 1-2 required params (like dag_id) are positional, rest stay as --flag required=True. Common CLI pattern (like git checkout <branch> --force). No hacks, all argparse features work.

I'd recommend Option 1 for simplicity. Happy to go with whichever you prefer!

---

Details on the parse_intermixed_args workaround (and why I didn't recommend it)

The idea is to register all required params as --flags only (no argparse positionals), add a single nargs='*' argument to collect any bare values, then map those bare values into whichever flag slots the user didn't fill:

PARAM_ORDER= ['dag_id', 'task_id', 'key']
parser.add_argument('--dag-id', dest='dag_id', default=None)
parser.add_argument('--task-id', dest='task_id', default=None)
parser.add_argument('--key', default=None)
parser.add_argument('positionals', nargs='*')
args=parser.parse_intermixed_args(argv)
# map bare values into unfilled flag slots, left to rightunfilled= [dfordinPARAM_ORDERifgetattr(args, d) isNone]
fori, valinenumerate(args.positionals):
setattr(args, unfilled[i], val)

parse_intermixed_args (Python 3.7+) allows flags and bare values to be freely interspersed, so all of these would work:

airflowctl xcom add d t k # all positional
airflowctl xcom add --dag-id=d --task-id=t --key=k # all flags
airflowctl xcom add d --task-id=t k # mixed
airflowctl xcom add --task-id=t my_dag my_key # gap-filling

The problem is what we lose:

• Validation — can't use required=True on flags (argparse would reject positional-only usage), so we validate manually after gap-filling
• Type checking — type=int, choices=[...] etc. don't apply to the nargs='*' catch-all, so we type-check manually
• Help output — shows [positionals ...] instead of named params like DAG_ID TASK_ID, and no indication which flags are required vs optional
• argparse.REMAINDER and mutually exclusive groups with positionals — unsupported with parse_intermixed_args

This effectively means using argparse for flag parsing only and reimplementing positional handling on top of it.

[bugraoz93]

@shivaam, this will make things a bit complicated. The two-pass parsing is still complex. I would say before we finish adding the command to the parser, we can reprocess the args, maybe even in get_parser, which can also be responsible for parsing the command. We may need to update the building part a bit but it should be possible with reprocessing

Even though we are still around 0.x.x, still thinking about how we could implement this in the best position. As it will be a user-facing change, we need to create a devlist discussion and ask to only keep positional for required or maintain both.

My idea is adding both can save us from the entire discussion flow and we could be backwards compatible, even though this is not trivial with argparase in Python.

[bugraoz93]

Something like this
#65261

[shivaam]

Closing in favor of #65261 which takes a better approach — supporting both positional and flag styles simultaneously via argv pre-processing, so there's no breaking change for existing users. Thanks @bugraoz93 for the guidance!

[shivaam]

Something like this #65261

I was just thinking about this. Supporting both positional and flag styles for the same param isn't something most CLI libraries or popular tools do. We're adding maintenance burden, constraining future changes as all CLI libraries might not suppor this feature, and expanding the testing surface forever. Since airflowctl is still pre-1.0, feels like this is the natural window for a clean break rather than committing to maintaining a non-standard approach indefinitely.

Since the layer we are adding is light, either way works. Just wanted to share my thoughts.

[bugraoz93]

Thanks @shivaam for your patience and joining the discussion! We have some procedures to follow as a community. :) Reopened yours, feel free to rebase and if ready we can ship

[bugraoz93]

https://lists.apache.org/thread/m1qvcvow3l17ytv40vhslh40wn3rntrm

[shivaam]

Sounds good. I will rebase the project.

[shivaam]

Before this PR, airflowctl backfill list (no args) returned [] and exited 0, but that was a silent bug — the CLI passed dag_id=None, which the server coerced to an empty filter that matched no records. The command was never actually returning backfills.

[bugraoz93]

Looks good! Thanks for your patience and work @shivaam!

[bugraoz93]

Added the full test label to see the integration tests green. We can merge after seeing it green :)

[bugraoz93]

CI looks unrelated, ctl test looks good

[bugraoz93]

@shivaam Could you please rebase and resolve conflicts, we can merge afterwards

[shivaam]

@bugraoz93 Seems like there is another PR that already solved this problem PR #66768 — "airflowctl: make required CLI params positional, keep optional as --flag" by 1fanwang
I dont think we need this anymore.

[bugraoz93]

Sorry @shivaam! Totally missed that one. I will try to be faster next time 😅 It also has some release notes changes need manually handled while backporting and should be cleaned while generating

[shivaam]

Required non-boolean parameters in auto-generated CLI commands are now positional instead of flag-style, improving UX consistency with manually defined commands like dags pause.

Before:

airflowctl connections create --connection-id="test" --conn-type="mysql" --password=secret
airflowctl dags get --dag-id=example_bash_operator

After:

airflowctl connections create test mysql --password=secret
airflowctl dags get example_bash_operator

Changes

• Track which operation parameters have default values during AST parsing
• For primitive params, use the default info to decide positional vs flag
• For Pydantic model fields, use is_required() to decide positional vs flag
• Use _UNSET for arg_dest default to avoid argparse crash on positional args
• Skip the has_default metadata key at runtime when mapping CLI args to method params
• Updated unit tests and integration test commands to match new positional syntax
• Added 2 new tests for positional arg behavior and default detection
• Added newsfragment for the breaking change

Notes

• _create_arg stays a dumb pass-through builder — all positional/flag logic is in the callers via one-liner conditionals
• Booleans are never positional (they need --flag/--no-flag)
• No special-casing for specific operations
• Breaking change: existing --flag=value syntax for required params will no longer work. Since airflowctl is new and not yet stable, this should be acceptable.

Open question

Posted a clarifying question on the issue about whether all required params should be positional (e.g., xcom add has 5 positional args), or if we should limit it.

closes: #60142

---

Was generative AI tooling used to co-author this PR?

• Yes — Claude Code (Claude Opus 4.6)

Generated-by: Claude Code (Claude Opus 4.6) following the guidelines

[bugraoz93]

Thanks for the PR! I addressed your question. Could you please amend it to be both optional and positional, live side by side?
This way, we won't break anything while working, but rather add Airflow CLI positional arguments to improve parity and make migrations easier.

[bugraoz93]

This news fragment is for Airflow Core and not related to anything within airflowctl flow.
Since we are still in 0.x, significant changes are also expected at a certain level.
We are logging these things in the release notes

https://airflow.apache.org/docs/apache-airflow-ctl/stable/release_notes.html#significant-changes

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates auto-generated airflowctl commands so required non-boolean parameters are positional (UX aligned with manually-defined commands), and adjusts parsing/tests to support the breaking CLI syntax change.

Changes:

• Track whether operation parameters have defaults during AST parsing and use it to decide positional vs flag args for primitive params.
• Use Pydantic model_fields[].is_required() to decide positional vs flag args for datamodel fields (booleans remain flags).
• Update unit/integration tests and add a newsfragment documenting the breaking change.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py	Updates assertions for positional args; adds tests for positional behavior and default detection.
airflow-ctl/src/airflowctl/ctl/cli_config.py	Adds has_default metadata via AST parsing; generates positional args for required params; skips metadata during arg mapping and invocation.
airflow-ctl-tests/tests/airflowctl_tests/test_config_sensitive_masking.py	Updates integration test commands to new positional syntax.
airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py	Updates end-to-end command list to match new positional syntax across many commands.
airflow-core/newsfragments/64812.significant.rst	Documents the breaking CLI change for auto-generated commands.

[Copilot]

The parameters entries are now shaped as a dict with a dynamic key for the param name plus a reserved has_default key. This forces downstream code to iterate parameter.items() and special-case parameter_key == \"has_default\", which is brittle and harder to extend (and also makes collisions with a real param named has_default difficult to reason about). Consider switching to a stable structure like { \"name\": ..., \"type\": ..., \"has_default\": ... } for each parameter so consumers can access fields directly without item-iteration + skips.

Suggested change

	args.append({arg_name: arg_type, "has_default": has_default})
	args.append(
	{"name": arg_name, "type": arg_type, "has_default": has_default}
	)

[Copilot]

Related to the parameter-shape issue: the for parameter_key, parameter_type in parameter.items() loop plus if parameter_key == \"has_default\": continue is an avoidable control-flow hack that spreads across multiple call sites (also in _get_func). If you keep the current shape, a safer alternative is to store metadata under a dedicated nested key (e.g. {\"name\": ..., \"type\": ..., \"meta\": {\"has_default\": ...}}) or move metadata out of the per-parameter dict entirely so this loop can be simplified and cannot accidentally skip real parameters.

[Copilot]

This test writes a fixed filename (test_has_default.py) into the working directory, which can conflict under parallel test runs or when the CWD is not writable. Use pytest’s tmp_path/tmp_path_factory (or tempfile.NamedTemporaryFile) to create an isolated temporary file path, and open it with an explicit encoding (e.g. encoding=\"utf-8\") to match the production file-read behavior.

Suggested change

	deftest_has_default_detection_in_ast_parsing(self):
	"""Test that AST parsing correctly detects which params have defaults."""
	fromtextwrapimportdedent
	temp_file="test_has_default.py"
	withopen(temp_file, "w") asf:
	f.write(
	dedent("""
	classTestOperations(BaseOperations):
	defget(self, required_id: str) ->str|ServerResponseError:
	pass
	defsearch(self, query: str, limit: int=10, offset: int=0) ->str|ServerResponseError:
	pass
	""")
	)
	try:
	command_factory=CommandFactory(file_path=temp_file)
	foropincommand_factory.operations:
	ifop["name"] =="get":
	param=op["parameters"][0]
	assertparam.get("has_default") isFalse
	elifop["name"] =="search":
	query_param=op["parameters"][0]
	assertquery_param.get("has_default") isFalse
	limit_param=op["parameters"][1]
	assertlimit_param.get("has_default") isTrue
	offset_param=op["parameters"][2]
	assertoffset_param.get("has_default") isTrue
	finally:
	importos
	os.remove(temp_file)
	deftest_has_default_detection_in_ast_parsing(self, tmp_path):
	"""Test that AST parsing correctly detects which params have defaults."""
	fromtextwrapimportdedent
	temp_file=tmp_path/"test_has_default.py"
	temp_file.write_text(
	dedent("""
	classTestOperations(BaseOperations):
	defget(self, required_id: str) ->str|ServerResponseError:
	pass
	defsearch(self, query: str, limit: int=10, offset: int=0) ->str|ServerResponseError:
	pass
	"""),
	encoding="utf-8",
	)
	command_factory=CommandFactory(file_path=str(temp_file))
	foropincommand_factory.operations:
	ifop["name"] =="get":
	param=op["parameters"][0]
	assertparam.get("has_default") isFalse
	elifop["name"] =="search":
	query_param=op["parameters"][0]
	assertquery_param.get("has_default") isFalse
	limit_param=op["parameters"][1]
	assertlimit_param.get("has_default") isTrue
	offset_param=op["parameters"][2]
	assertoffset_param.get("has_default") isTrue

[bugraoz93]

Moving to draft to manage release better, please make it ready again when you think ready for maintainers review

[shivaam]

Moving to draft to manage release better, please make it ready again when you think ready for maintainers review
@bugraoz93!

Sorry I replied on the issue instead of the PR. argparse doesn't natively support this. Below I have listed some options.

---

I looked into supporting both positional and --flag styles for required params. Unfortunately argparse doesn't natively support this — a parameter is either positional (dag_id) or a flag (--dag-id), never both.

Why it's not straightforward:

The closest workaround is registering all params as --flags, adding a nargs='*' catch-all for bare values, and using parse_intermixed_args to map positionals into unfilled flag slots after parsing. More about this at the bottom of this comment. But this means we lose argparse's built-in validation, type checking, help formatting, and required=True for those params — we'd essentially reimplement positional argument handling ourselves on top of argparse.

Options:

1. Keep it positional for required params (current PR) — the simplest, most conventional choice. Required params are positional, optional params are flags. All argparse features work, clean help output.

2. Two-pass parsing — maintain two parsers per command: one with required params as positional, one with them as --flags required=True. Try positional first, fall back to flags. Users can use either style, just not mixed in the same invocation:

   # both work:
   airflowctl xcom add my_dag my_task my_key
   airflowctl xcom add --dag-id=my_dag --task-id=my_task --key=my_key
   # but not mixed:
   airflowctl xcom add my_dag --task-id=my_task my_key # would fail

   No argparse hacks needed (both parsers are standard), but doubles the parser setup and mixed usage isn't supported.

3. Cap positional count — first 1-2 required params (like dag_id) are positional, rest stay as --flag required=True. Common CLI pattern (like git checkout <branch> --force). No hacks, all argparse features work.

I'd recommend Option 1 for simplicity. Happy to go with whichever you prefer!

---

Details on the parse_intermixed_args workaround (and why I didn't recommend it)

The idea is to register all required params as --flags only (no argparse positionals), add a single nargs='*' argument to collect any bare values, then map those bare values into whichever flag slots the user didn't fill:

PARAM_ORDER= ['dag_id', 'task_id', 'key']
parser.add_argument('--dag-id', dest='dag_id', default=None)
parser.add_argument('--task-id', dest='task_id', default=None)
parser.add_argument('--key', default=None)
parser.add_argument('positionals', nargs='*')
args=parser.parse_intermixed_args(argv)
# map bare values into unfilled flag slots, left to rightunfilled= [dfordinPARAM_ORDERifgetattr(args, d) isNone]
fori, valinenumerate(args.positionals):
setattr(args, unfilled[i], val)

parse_intermixed_args (Python 3.7+) allows flags and bare values to be freely interspersed, so all of these would work:

airflowctl xcom add d t k # all positional
airflowctl xcom add --dag-id=d --task-id=t --key=k # all flags
airflowctl xcom add d --task-id=t k # mixed
airflowctl xcom add --task-id=t my_dag my_key # gap-filling

The problem is what we lose:

• Validation — can't use required=True on flags (argparse would reject positional-only usage), so we validate manually after gap-filling
• Type checking — type=int, choices=[...] etc. don't apply to the nargs='*' catch-all, so we type-check manually
• Help output — shows [positionals ...] instead of named params like DAG_ID TASK_ID, and no indication which flags are required vs optional
• argparse.REMAINDER and mutually exclusive groups with positionals — unsupported with parse_intermixed_args

This effectively means using argparse for flag parsing only and reimplementing positional handling on top of it.

[bugraoz93]

@shivaam, this will make things a bit complicated. The two-pass parsing is still complex. I would say before we finish adding the command to the parser, we can reprocess the args, maybe even in get_parser, which can also be responsible for parsing the command. We may need to update the building part a bit but it should be possible with reprocessing

Even though we are still around 0.x.x, still thinking about how we could implement this in the best position. As it will be a user-facing change, we need to create a devlist discussion and ask to only keep positional for required or maintain both.

My idea is adding both can save us from the entire discussion flow and we could be backwards compatible, even though this is not trivial with argparase in Python.

[bugraoz93]

Something like this
#65261

[shivaam]

Closing in favor of #65261 which takes a better approach — supporting both positional and flag styles simultaneously via argv pre-processing, so there's no breaking change for existing users. Thanks @bugraoz93 for the guidance!

[shivaam]

Something like this #65261

I was just thinking about this. Supporting both positional and flag styles for the same param isn't something most CLI libraries or popular tools do. We're adding maintenance burden, constraining future changes as all CLI libraries might not suppor this feature, and expanding the testing surface forever. Since airflowctl is still pre-1.0, feels like this is the natural window for a clean break rather than committing to maintaining a non-standard approach indefinitely.

Since the layer we are adding is light, either way works. Just wanted to share my thoughts.

[bugraoz93]

Thanks @shivaam for your patience and joining the discussion! We have some procedures to follow as a community. :) Reopened yours, feel free to rebase and if ready we can ship

[bugraoz93]

https://lists.apache.org/thread/m1qvcvow3l17ytv40vhslh40wn3rntrm

[shivaam]

Sounds good. I will rebase the project.

[shivaam]

Before this PR, airflowctl backfill list (no args) returned [] and exited 0, but that was a silent bug — the CLI passed dag_id=None, which the server coerced to an empty filter that matched no records. The command was never actually returning backfills.

[bugraoz93]

Looks good! Thanks for your patience and work @shivaam!

[bugraoz93]

Added the full test label to see the integration tests green. We can merge after seeing it green :)

[bugraoz93]

CI looks unrelated, ctl test looks good

[bugraoz93]

@shivaam Could you please rebase and resolve conflicts, we can merge afterwards

[shivaam]

@bugraoz93 Seems like there is another PR that already solved this problem PR #66768 — "airflowctl: make required CLI params positional, keep optional as --flag" by 1fanwang
I dont think we need this anymore.

[bugraoz93]

Sorry @shivaam! Totally missed that one. I will try to be faster next time 😅 It also has some release notes changes need manually handled while backporting and should be cleaned while generating

[shivaam]

Required non-boolean parameters in auto-generated CLI commands are now positional instead of flag-style, improving UX consistency with manually defined commands like dags pause.

Before:

airflowctl connections create --connection-id="test" --conn-type="mysql" --password=secret
airflowctl dags get --dag-id=example_bash_operator

After:

airflowctl connections create test mysql --password=secret
airflowctl dags get example_bash_operator

Changes

• Track which operation parameters have default values during AST parsing
• For primitive params, use the default info to decide positional vs flag
• For Pydantic model fields, use is_required() to decide positional vs flag
• Use _UNSET for arg_dest default to avoid argparse crash on positional args
• Skip the has_default metadata key at runtime when mapping CLI args to method params
• Updated unit tests and integration test commands to match new positional syntax
• Added 2 new tests for positional arg behavior and default detection
• Added newsfragment for the breaking change

Notes

• _create_arg stays a dumb pass-through builder — all positional/flag logic is in the callers via one-liner conditionals
• Booleans are never positional (they need --flag/--no-flag)
• No special-casing for specific operations
• Breaking change: existing --flag=value syntax for required params will no longer work. Since airflowctl is new and not yet stable, this should be acceptable.

Open question

Posted a clarifying question on the issue about whether all required params should be positional (e.g., xcom add has 5 positional args), or if we should limit it.

closes: #60142

---

Was generative AI tooling used to co-author this PR?

• Yes — Claude Code (Claude Opus 4.6)

Generated-by: Claude Code (Claude Opus 4.6) following the guidelines

[bugraoz93]

Thanks for the PR! I addressed your question. Could you please amend it to be both optional and positional, live side by side?
This way, we won't break anything while working, but rather add Airflow CLI positional arguments to improve parity and make migrations easier.

[bugraoz93]

This news fragment is for Airflow Core and not related to anything within airflowctl flow.
Since we are still in 0.x, significant changes are also expected at a certain level.
We are logging these things in the release notes

https://airflow.apache.org/docs/apache-airflow-ctl/stable/release_notes.html#significant-changes

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates auto-generated airflowctl commands so required non-boolean parameters are positional (UX aligned with manually-defined commands), and adjusts parsing/tests to support the breaking CLI syntax change.

Changes:

• Track whether operation parameters have defaults during AST parsing and use it to decide positional vs flag args for primitive params.
• Use Pydantic model_fields[].is_required() to decide positional vs flag args for datamodel fields (booleans remain flags).
• Update unit/integration tests and add a newsfragment documenting the breaking change.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py	Updates assertions for positional args; adds tests for positional behavior and default detection.
airflow-ctl/src/airflowctl/ctl/cli_config.py	Adds has_default metadata via AST parsing; generates positional args for required params; skips metadata during arg mapping and invocation.
airflow-ctl-tests/tests/airflowctl_tests/test_config_sensitive_masking.py	Updates integration test commands to new positional syntax.
airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py	Updates end-to-end command list to match new positional syntax across many commands.
airflow-core/newsfragments/64812.significant.rst	Documents the breaking CLI change for auto-generated commands.

[Copilot]

The parameters entries are now shaped as a dict with a dynamic key for the param name plus a reserved has_default key. This forces downstream code to iterate parameter.items() and special-case parameter_key == \"has_default\", which is brittle and harder to extend (and also makes collisions with a real param named has_default difficult to reason about). Consider switching to a stable structure like { \"name\": ..., \"type\": ..., \"has_default\": ... } for each parameter so consumers can access fields directly without item-iteration + skips.

Suggested change

	args.append({arg_name: arg_type, "has_default": has_default})
	args.append(
	{"name": arg_name, "type": arg_type, "has_default": has_default}
	)

[Copilot]

Related to the parameter-shape issue: the for parameter_key, parameter_type in parameter.items() loop plus if parameter_key == \"has_default\": continue is an avoidable control-flow hack that spreads across multiple call sites (also in _get_func). If you keep the current shape, a safer alternative is to store metadata under a dedicated nested key (e.g. {\"name\": ..., \"type\": ..., \"meta\": {\"has_default\": ...}}) or move metadata out of the per-parameter dict entirely so this loop can be simplified and cannot accidentally skip real parameters.

[Copilot]

This test writes a fixed filename (test_has_default.py) into the working directory, which can conflict under parallel test runs or when the CWD is not writable. Use pytest’s tmp_path/tmp_path_factory (or tempfile.NamedTemporaryFile) to create an isolated temporary file path, and open it with an explicit encoding (e.g. encoding=\"utf-8\") to match the production file-read behavior.

Suggested change

	deftest_has_default_detection_in_ast_parsing(self):
	"""Test that AST parsing correctly detects which params have defaults."""
	fromtextwrapimportdedent
	temp_file="test_has_default.py"
	withopen(temp_file, "w") asf:
	f.write(
	dedent("""
	classTestOperations(BaseOperations):
	defget(self, required_id: str) ->str|ServerResponseError:
	pass
	defsearch(self, query: str, limit: int=10, offset: int=0) ->str|ServerResponseError:
	pass
	""")
	)
	try:
	command_factory=CommandFactory(file_path=temp_file)
	foropincommand_factory.operations:
	ifop["name"] =="get":
	param=op["parameters"][0]
	assertparam.get("has_default") isFalse
	elifop["name"] =="search":
	query_param=op["parameters"][0]
	assertquery_param.get("has_default") isFalse
	limit_param=op["parameters"][1]
	assertlimit_param.get("has_default") isTrue
	offset_param=op["parameters"][2]
	assertoffset_param.get("has_default") isTrue
	finally:
	importos
	os.remove(temp_file)
	deftest_has_default_detection_in_ast_parsing(self, tmp_path):
	"""Test that AST parsing correctly detects which params have defaults."""
	fromtextwrapimportdedent
	temp_file=tmp_path/"test_has_default.py"
	temp_file.write_text(
	dedent("""
	classTestOperations(BaseOperations):
	defget(self, required_id: str) ->str|ServerResponseError:
	pass
	defsearch(self, query: str, limit: int=10, offset: int=0) ->str|ServerResponseError:
	pass
	"""),
	encoding="utf-8",
	)
	command_factory=CommandFactory(file_path=str(temp_file))
	foropincommand_factory.operations:
	ifop["name"] =="get":
	param=op["parameters"][0]
	assertparam.get("has_default") isFalse
	elifop["name"] =="search":
	query_param=op["parameters"][0]
	assertquery_param.get("has_default") isFalse
	limit_param=op["parameters"][1]
	assertlimit_param.get("has_default") isTrue
	offset_param=op["parameters"][2]
	assertoffset_param.get("has_default") isTrue

[bugraoz93]

Moving to draft to manage release better, please make it ready again when you think ready for maintainers review

[shivaam]

Moving to draft to manage release better, please make it ready again when you think ready for maintainers review
@bugraoz93!

Sorry I replied on the issue instead of the PR. argparse doesn't natively support this. Below I have listed some options.

---

I looked into supporting both positional and --flag styles for required params. Unfortunately argparse doesn't natively support this — a parameter is either positional (dag_id) or a flag (--dag-id), never both.

Why it's not straightforward:

The closest workaround is registering all params as --flags, adding a nargs='*' catch-all for bare values, and using parse_intermixed_args to map positionals into unfilled flag slots after parsing. More about this at the bottom of this comment. But this means we lose argparse's built-in validation, type checking, help formatting, and required=True for those params — we'd essentially reimplement positional argument handling ourselves on top of argparse.

Options:

1. Keep it positional for required params (current PR) — the simplest, most conventional choice. Required params are positional, optional params are flags. All argparse features work, clean help output.

2. Two-pass parsing — maintain two parsers per command: one with required params as positional, one with them as --flags required=True. Try positional first, fall back to flags. Users can use either style, just not mixed in the same invocation:

   # both work:
   airflowctl xcom add my_dag my_task my_key
   airflowctl xcom add --dag-id=my_dag --task-id=my_task --key=my_key
   # but not mixed:
   airflowctl xcom add my_dag --task-id=my_task my_key # would fail

   No argparse hacks needed (both parsers are standard), but doubles the parser setup and mixed usage isn't supported.

3. Cap positional count — first 1-2 required params (like dag_id) are positional, rest stay as --flag required=True. Common CLI pattern (like git checkout <branch> --force). No hacks, all argparse features work.

I'd recommend Option 1 for simplicity. Happy to go with whichever you prefer!

---

Details on the parse_intermixed_args workaround (and why I didn't recommend it)

The idea is to register all required params as --flags only (no argparse positionals), add a single nargs='*' argument to collect any bare values, then map those bare values into whichever flag slots the user didn't fill:

PARAM_ORDER= ['dag_id', 'task_id', 'key']
parser.add_argument('--dag-id', dest='dag_id', default=None)
parser.add_argument('--task-id', dest='task_id', default=None)
parser.add_argument('--key', default=None)
parser.add_argument('positionals', nargs='*')
args=parser.parse_intermixed_args(argv)
# map bare values into unfilled flag slots, left to rightunfilled= [dfordinPARAM_ORDERifgetattr(args, d) isNone]
fori, valinenumerate(args.positionals):
setattr(args, unfilled[i], val)

parse_intermixed_args (Python 3.7+) allows flags and bare values to be freely interspersed, so all of these would work:

airflowctl xcom add d t k # all positional
airflowctl xcom add --dag-id=d --task-id=t --key=k # all flags
airflowctl xcom add d --task-id=t k # mixed
airflowctl xcom add --task-id=t my_dag my_key # gap-filling

The problem is what we lose:

• Validation — can't use required=True on flags (argparse would reject positional-only usage), so we validate manually after gap-filling
• Type checking — type=int, choices=[...] etc. don't apply to the nargs='*' catch-all, so we type-check manually
• Help output — shows [positionals ...] instead of named params like DAG_ID TASK_ID, and no indication which flags are required vs optional
• argparse.REMAINDER and mutually exclusive groups with positionals — unsupported with parse_intermixed_args

This effectively means using argparse for flag parsing only and reimplementing positional handling on top of it.

[bugraoz93]

@shivaam, this will make things a bit complicated. The two-pass parsing is still complex. I would say before we finish adding the command to the parser, we can reprocess the args, maybe even in get_parser, which can also be responsible for parsing the command. We may need to update the building part a bit but it should be possible with reprocessing

Even though we are still around 0.x.x, still thinking about how we could implement this in the best position. As it will be a user-facing change, we need to create a devlist discussion and ask to only keep positional for required or maintain both.

My idea is adding both can save us from the entire discussion flow and we could be backwards compatible, even though this is not trivial with argparase in Python.

[bugraoz93]

Something like this
#65261

[shivaam]

Closing in favor of #65261 which takes a better approach — supporting both positional and flag styles simultaneously via argv pre-processing, so there's no breaking change for existing users. Thanks @bugraoz93 for the guidance!

[shivaam]

Something like this #65261

I was just thinking about this. Supporting both positional and flag styles for the same param isn't something most CLI libraries or popular tools do. We're adding maintenance burden, constraining future changes as all CLI libraries might not suppor this feature, and expanding the testing surface forever. Since airflowctl is still pre-1.0, feels like this is the natural window for a clean break rather than committing to maintaining a non-standard approach indefinitely.

Since the layer we are adding is light, either way works. Just wanted to share my thoughts.

[bugraoz93]

Thanks @shivaam for your patience and joining the discussion! We have some procedures to follow as a community. :) Reopened yours, feel free to rebase and if ready we can ship

[bugraoz93]

https://lists.apache.org/thread/m1qvcvow3l17ytv40vhslh40wn3rntrm

[shivaam]

Sounds good. I will rebase the project.

[shivaam]

Before this PR, airflowctl backfill list (no args) returned [] and exited 0, but that was a silent bug — the CLI passed dag_id=None, which the server coerced to an empty filter that matched no records. The command was never actually returning backfills.

[bugraoz93]

Looks good! Thanks for your patience and work @shivaam!

[bugraoz93]

Added the full test label to see the integration tests green. We can merge after seeing it green :)

[bugraoz93]

CI looks unrelated, ctl test looks good

[bugraoz93]

@shivaam Could you please rebase and resolve conflicts, we can merge afterwards

[shivaam]

@bugraoz93 Seems like there is another PR that already solved this problem PR #66768 — "airflowctl: make required CLI params positional, keep optional as --flag" by 1fanwang
I dont think we need this anymore.

[bugraoz93]

Sorry @shivaam! Totally missed that one. I will try to be faster next time 😅 It also has some release notes changes need manually handled while backporting and should be cleaned while generating

[shivaam]

Required non-boolean parameters in auto-generated CLI commands are now positional instead of flag-style, improving UX consistency with manually defined commands like dags pause.

Before:

airflowctl connections create --connection-id="test" --conn-type="mysql" --password=secret
airflowctl dags get --dag-id=example_bash_operator

After:

airflowctl connections create test mysql --password=secret
airflowctl dags get example_bash_operator

Changes

• Track which operation parameters have default values during AST parsing
• For primitive params, use the default info to decide positional vs flag
• For Pydantic model fields, use is_required() to decide positional vs flag
• Use _UNSET for arg_dest default to avoid argparse crash on positional args
• Skip the has_default metadata key at runtime when mapping CLI args to method params
• Updated unit tests and integration test commands to match new positional syntax
• Added 2 new tests for positional arg behavior and default detection
• Added newsfragment for the breaking change

Notes

• _create_arg stays a dumb pass-through builder — all positional/flag logic is in the callers via one-liner conditionals
• Booleans are never positional (they need --flag/--no-flag)
• No special-casing for specific operations
• Breaking change: existing --flag=value syntax for required params will no longer work. Since airflowctl is new and not yet stable, this should be acceptable.

Open question

Posted a clarifying question on the issue about whether all required params should be positional (e.g., xcom add has 5 positional args), or if we should limit it.

closes: #60142

---

Was generative AI tooling used to co-author this PR?

• Yes — Claude Code (Claude Opus 4.6)

Generated-by: Claude Code (Claude Opus 4.6) following the guidelines

[bugraoz93]

Thanks for the PR! I addressed your question. Could you please amend it to be both optional and positional, live side by side?
This way, we won't break anything while working, but rather add Airflow CLI positional arguments to improve parity and make migrations easier.

[bugraoz93]

This news fragment is for Airflow Core and not related to anything within airflowctl flow.
Since we are still in 0.x, significant changes are also expected at a certain level.
We are logging these things in the release notes

https://airflow.apache.org/docs/apache-airflow-ctl/stable/release_notes.html#significant-changes

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates auto-generated airflowctl commands so required non-boolean parameters are positional (UX aligned with manually-defined commands), and adjusts parsing/tests to support the breaking CLI syntax change.

Changes:

• Track whether operation parameters have defaults during AST parsing and use it to decide positional vs flag args for primitive params.
• Use Pydantic model_fields[].is_required() to decide positional vs flag args for datamodel fields (booleans remain flags).
• Update unit/integration tests and add a newsfragment documenting the breaking change.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py	Updates assertions for positional args; adds tests for positional behavior and default detection.
airflow-ctl/src/airflowctl/ctl/cli_config.py	Adds has_default metadata via AST parsing; generates positional args for required params; skips metadata during arg mapping and invocation.
airflow-ctl-tests/tests/airflowctl_tests/test_config_sensitive_masking.py	Updates integration test commands to new positional syntax.
airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py	Updates end-to-end command list to match new positional syntax across many commands.
airflow-core/newsfragments/64812.significant.rst	Documents the breaking CLI change for auto-generated commands.

[Copilot]

The parameters entries are now shaped as a dict with a dynamic key for the param name plus a reserved has_default key. This forces downstream code to iterate parameter.items() and special-case parameter_key == \"has_default\", which is brittle and harder to extend (and also makes collisions with a real param named has_default difficult to reason about). Consider switching to a stable structure like { \"name\": ..., \"type\": ..., \"has_default\": ... } for each parameter so consumers can access fields directly without item-iteration + skips.

Suggested change

	args.append({arg_name: arg_type, "has_default": has_default})
	args.append(
	{"name": arg_name, "type": arg_type, "has_default": has_default}
	)

[Copilot]

Related to the parameter-shape issue: the for parameter_key, parameter_type in parameter.items() loop plus if parameter_key == \"has_default\": continue is an avoidable control-flow hack that spreads across multiple call sites (also in _get_func). If you keep the current shape, a safer alternative is to store metadata under a dedicated nested key (e.g. {\"name\": ..., \"type\": ..., \"meta\": {\"has_default\": ...}}) or move metadata out of the per-parameter dict entirely so this loop can be simplified and cannot accidentally skip real parameters.

[Copilot]

This test writes a fixed filename (test_has_default.py) into the working directory, which can conflict under parallel test runs or when the CWD is not writable. Use pytest’s tmp_path/tmp_path_factory (or tempfile.NamedTemporaryFile) to create an isolated temporary file path, and open it with an explicit encoding (e.g. encoding=\"utf-8\") to match the production file-read behavior.

Suggested change

	deftest_has_default_detection_in_ast_parsing(self):
	"""Test that AST parsing correctly detects which params have defaults."""
	fromtextwrapimportdedent
	temp_file="test_has_default.py"
	withopen(temp_file, "w") asf:
	f.write(
	dedent("""
	classTestOperations(BaseOperations):
	defget(self, required_id: str) ->str|ServerResponseError:
	pass
	defsearch(self, query: str, limit: int=10, offset: int=0) ->str|ServerResponseError:
	pass
	""")
	)
	try:
	command_factory=CommandFactory(file_path=temp_file)
	foropincommand_factory.operations:
	ifop["name"] =="get":
	param=op["parameters"][0]
	assertparam.get("has_default") isFalse
	elifop["name"] =="search":
	query_param=op["parameters"][0]
	assertquery_param.get("has_default") isFalse
	limit_param=op["parameters"][1]
	assertlimit_param.get("has_default") isTrue
	offset_param=op["parameters"][2]
	assertoffset_param.get("has_default") isTrue
	finally:
	importos
	os.remove(temp_file)
	deftest_has_default_detection_in_ast_parsing(self, tmp_path):
	"""Test that AST parsing correctly detects which params have defaults."""
	fromtextwrapimportdedent
	temp_file=tmp_path/"test_has_default.py"
	temp_file.write_text(
	dedent("""
	classTestOperations(BaseOperations):
	defget(self, required_id: str) ->str|ServerResponseError:
	pass
	defsearch(self, query: str, limit: int=10, offset: int=0) ->str|ServerResponseError:
	pass
	"""),
	encoding="utf-8",
	)
	command_factory=CommandFactory(file_path=str(temp_file))
	foropincommand_factory.operations:
	ifop["name"] =="get":
	param=op["parameters"][0]
	assertparam.get("has_default") isFalse
	elifop["name"] =="search":
	query_param=op["parameters"][0]
	assertquery_param.get("has_default") isFalse
	limit_param=op["parameters"][1]
	assertlimit_param.get("has_default") isTrue
	offset_param=op["parameters"][2]
	assertoffset_param.get("has_default") isTrue

[bugraoz93]

Moving to draft to manage release better, please make it ready again when you think ready for maintainers review

[shivaam]

Moving to draft to manage release better, please make it ready again when you think ready for maintainers review
@bugraoz93!

Sorry I replied on the issue instead of the PR. argparse doesn't natively support this. Below I have listed some options.

---

I looked into supporting both positional and --flag styles for required params. Unfortunately argparse doesn't natively support this — a parameter is either positional (dag_id) or a flag (--dag-id), never both.

Why it's not straightforward:

The closest workaround is registering all params as --flags, adding a nargs='*' catch-all for bare values, and using parse_intermixed_args to map positionals into unfilled flag slots after parsing. More about this at the bottom of this comment. But this means we lose argparse's built-in validation, type checking, help formatting, and required=True for those params — we'd essentially reimplement positional argument handling ourselves on top of argparse.

Options:

1. Keep it positional for required params (current PR) — the simplest, most conventional choice. Required params are positional, optional params are flags. All argparse features work, clean help output.

2. Two-pass parsing — maintain two parsers per command: one with required params as positional, one with them as --flags required=True. Try positional first, fall back to flags. Users can use either style, just not mixed in the same invocation:

   # both work:
   airflowctl xcom add my_dag my_task my_key
   airflowctl xcom add --dag-id=my_dag --task-id=my_task --key=my_key
   # but not mixed:
   airflowctl xcom add my_dag --task-id=my_task my_key # would fail

   No argparse hacks needed (both parsers are standard), but doubles the parser setup and mixed usage isn't supported.

3. Cap positional count — first 1-2 required params (like dag_id) are positional, rest stay as --flag required=True. Common CLI pattern (like git checkout <branch> --force). No hacks, all argparse features work.

I'd recommend Option 1 for simplicity. Happy to go with whichever you prefer!

---

Details on the parse_intermixed_args workaround (and why I didn't recommend it)

The idea is to register all required params as --flags only (no argparse positionals), add a single nargs='*' argument to collect any bare values, then map those bare values into whichever flag slots the user didn't fill:

PARAM_ORDER= ['dag_id', 'task_id', 'key']
parser.add_argument('--dag-id', dest='dag_id', default=None)
parser.add_argument('--task-id', dest='task_id', default=None)
parser.add_argument('--key', default=None)
parser.add_argument('positionals', nargs='*')
args=parser.parse_intermixed_args(argv)
# map bare values into unfilled flag slots, left to rightunfilled= [dfordinPARAM_ORDERifgetattr(args, d) isNone]
fori, valinenumerate(args.positionals):
setattr(args, unfilled[i], val)

parse_intermixed_args (Python 3.7+) allows flags and bare values to be freely interspersed, so all of these would work:

airflowctl xcom add d t k # all positional
airflowctl xcom add --dag-id=d --task-id=t --key=k # all flags
airflowctl xcom add d --task-id=t k # mixed
airflowctl xcom add --task-id=t my_dag my_key # gap-filling

The problem is what we lose:

• Validation — can't use required=True on flags (argparse would reject positional-only usage), so we validate manually after gap-filling
• Type checking — type=int, choices=[...] etc. don't apply to the nargs='*' catch-all, so we type-check manually
• Help output — shows [positionals ...] instead of named params like DAG_ID TASK_ID, and no indication which flags are required vs optional
• argparse.REMAINDER and mutually exclusive groups with positionals — unsupported with parse_intermixed_args

This effectively means using argparse for flag parsing only and reimplementing positional handling on top of it.

[bugraoz93]

@shivaam, this will make things a bit complicated. The two-pass parsing is still complex. I would say before we finish adding the command to the parser, we can reprocess the args, maybe even in get_parser, which can also be responsible for parsing the command. We may need to update the building part a bit but it should be possible with reprocessing

Even though we are still around 0.x.x, still thinking about how we could implement this in the best position. As it will be a user-facing change, we need to create a devlist discussion and ask to only keep positional for required or maintain both.

My idea is adding both can save us from the entire discussion flow and we could be backwards compatible, even though this is not trivial with argparase in Python.

[bugraoz93]

Something like this
#65261

[shivaam]

Closing in favor of #65261 which takes a better approach — supporting both positional and flag styles simultaneously via argv pre-processing, so there's no breaking change for existing users. Thanks @bugraoz93 for the guidance!

[shivaam]

Something like this #65261

I was just thinking about this. Supporting both positional and flag styles for the same param isn't something most CLI libraries or popular tools do. We're adding maintenance burden, constraining future changes as all CLI libraries might not suppor this feature, and expanding the testing surface forever. Since airflowctl is still pre-1.0, feels like this is the natural window for a clean break rather than committing to maintaining a non-standard approach indefinitely.

Since the layer we are adding is light, either way works. Just wanted to share my thoughts.

[bugraoz93]

Thanks @shivaam for your patience and joining the discussion! We have some procedures to follow as a community. :) Reopened yours, feel free to rebase and if ready we can ship

[bugraoz93]

https://lists.apache.org/thread/m1qvcvow3l17ytv40vhslh40wn3rntrm

[shivaam]

Sounds good. I will rebase the project.

[shivaam]

Before this PR, airflowctl backfill list (no args) returned [] and exited 0, but that was a silent bug — the CLI passed dag_id=None, which the server coerced to an empty filter that matched no records. The command was never actually returning backfills.

[bugraoz93]

Looks good! Thanks for your patience and work @shivaam!

[bugraoz93]

Added the full test label to see the integration tests green. We can merge after seeing it green :)

[bugraoz93]

CI looks unrelated, ctl test looks good

[bugraoz93]

@shivaam Could you please rebase and resolve conflicts, we can merge afterwards

[shivaam]

@bugraoz93 Seems like there is another PR that already solved this problem PR #66768 — "airflowctl: make required CLI params positional, keep optional as --flag" by 1fanwang
I dont think we need this anymore.

[bugraoz93]

Sorry @shivaam! Totally missed that one. I will try to be faster next time 😅 It also has some release notes changes need manually handled while backporting and should be cleaned while generating

[shivaam]

Required non-boolean parameters in auto-generated CLI commands are now positional instead of flag-style, improving UX consistency with manually defined commands like dags pause.

Before:

airflowctl connections create --connection-id="test" --conn-type="mysql" --password=secret
airflowctl dags get --dag-id=example_bash_operator

After:

airflowctl connections create test mysql --password=secret
airflowctl dags get example_bash_operator

Changes

• Track which operation parameters have default values during AST parsing
• For primitive params, use the default info to decide positional vs flag
• For Pydantic model fields, use is_required() to decide positional vs flag
• Use _UNSET for arg_dest default to avoid argparse crash on positional args
• Skip the has_default metadata key at runtime when mapping CLI args to method params
• Updated unit tests and integration test commands to match new positional syntax
• Added 2 new tests for positional arg behavior and default detection
• Added newsfragment for the breaking change

Notes

• _create_arg stays a dumb pass-through builder — all positional/flag logic is in the callers via one-liner conditionals
• Booleans are never positional (they need --flag/--no-flag)
• No special-casing for specific operations
• Breaking change: existing --flag=value syntax for required params will no longer work. Since airflowctl is new and not yet stable, this should be acceptable.

Open question

Posted a clarifying question on the issue about whether all required params should be positional (e.g., xcom add has 5 positional args), or if we should limit it.

closes: #60142

---

Was generative AI tooling used to co-author this PR?

• Yes — Claude Code (Claude Opus 4.6)

Generated-by: Claude Code (Claude Opus 4.6) following the guidelines

[bugraoz93]

Thanks for the PR! I addressed your question. Could you please amend it to be both optional and positional, live side by side?
This way, we won't break anything while working, but rather add Airflow CLI positional arguments to improve parity and make migrations easier.

[bugraoz93]

This news fragment is for Airflow Core and not related to anything within airflowctl flow.
Since we are still in 0.x, significant changes are also expected at a certain level.
We are logging these things in the release notes

https://airflow.apache.org/docs/apache-airflow-ctl/stable/release_notes.html#significant-changes

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates auto-generated airflowctl commands so required non-boolean parameters are positional (UX aligned with manually-defined commands), and adjusts parsing/tests to support the breaking CLI syntax change.

Changes:

• Track whether operation parameters have defaults during AST parsing and use it to decide positional vs flag args for primitive params.
• Use Pydantic model_fields[].is_required() to decide positional vs flag args for datamodel fields (booleans remain flags).
• Update unit/integration tests and add a newsfragment documenting the breaking change.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py	Updates assertions for positional args; adds tests for positional behavior and default detection.
airflow-ctl/src/airflowctl/ctl/cli_config.py	Adds has_default metadata via AST parsing; generates positional args for required params; skips metadata during arg mapping and invocation.
airflow-ctl-tests/tests/airflowctl_tests/test_config_sensitive_masking.py	Updates integration test commands to new positional syntax.
airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py	Updates end-to-end command list to match new positional syntax across many commands.
airflow-core/newsfragments/64812.significant.rst	Documents the breaking CLI change for auto-generated commands.

[Copilot]

The parameters entries are now shaped as a dict with a dynamic key for the param name plus a reserved has_default key. This forces downstream code to iterate parameter.items() and special-case parameter_key == \"has_default\", which is brittle and harder to extend (and also makes collisions with a real param named has_default difficult to reason about). Consider switching to a stable structure like { \"name\": ..., \"type\": ..., \"has_default\": ... } for each parameter so consumers can access fields directly without item-iteration + skips.

Suggested change

	args.append({arg_name: arg_type, "has_default": has_default})
	args.append(
	{"name": arg_name, "type": arg_type, "has_default": has_default}
	)

[Copilot]

Related to the parameter-shape issue: the for parameter_key, parameter_type in parameter.items() loop plus if parameter_key == \"has_default\": continue is an avoidable control-flow hack that spreads across multiple call sites (also in _get_func). If you keep the current shape, a safer alternative is to store metadata under a dedicated nested key (e.g. {\"name\": ..., \"type\": ..., \"meta\": {\"has_default\": ...}}) or move metadata out of the per-parameter dict entirely so this loop can be simplified and cannot accidentally skip real parameters.

[Copilot]

This test writes a fixed filename (test_has_default.py) into the working directory, which can conflict under parallel test runs or when the CWD is not writable. Use pytest’s tmp_path/tmp_path_factory (or tempfile.NamedTemporaryFile) to create an isolated temporary file path, and open it with an explicit encoding (e.g. encoding=\"utf-8\") to match the production file-read behavior.

Suggested change

	deftest_has_default_detection_in_ast_parsing(self):
	"""Test that AST parsing correctly detects which params have defaults."""
	fromtextwrapimportdedent
	temp_file="test_has_default.py"
	withopen(temp_file, "w") asf:
	f.write(
	dedent("""
	classTestOperations(BaseOperations):
	defget(self, required_id: str) ->str|ServerResponseError:
	pass
	defsearch(self, query: str, limit: int=10, offset: int=0) ->str|ServerResponseError:
	pass
	""")
	)
	try:
	command_factory=CommandFactory(file_path=temp_file)
	foropincommand_factory.operations:
	ifop["name"] =="get":
	param=op["parameters"][0]
	assertparam.get("has_default") isFalse
	elifop["name"] =="search":
	query_param=op["parameters"][0]
	assertquery_param.get("has_default") isFalse
	limit_param=op["parameters"][1]
	assertlimit_param.get("has_default") isTrue
	offset_param=op["parameters"][2]
	assertoffset_param.get("has_default") isTrue
	finally:
	importos
	os.remove(temp_file)
	deftest_has_default_detection_in_ast_parsing(self, tmp_path):
	"""Test that AST parsing correctly detects which params have defaults."""
	fromtextwrapimportdedent
	temp_file=tmp_path/"test_has_default.py"
	temp_file.write_text(
	dedent("""
	classTestOperations(BaseOperations):
	defget(self, required_id: str) ->str|ServerResponseError:
	pass
	defsearch(self, query: str, limit: int=10, offset: int=0) ->str|ServerResponseError:
	pass
	"""),
	encoding="utf-8",
	)
	command_factory=CommandFactory(file_path=str(temp_file))
	foropincommand_factory.operations:
	ifop["name"] =="get":
	param=op["parameters"][0]
	assertparam.get("has_default") isFalse
	elifop["name"] =="search":
	query_param=op["parameters"][0]
	assertquery_param.get("has_default") isFalse
	limit_param=op["parameters"][1]
	assertlimit_param.get("has_default") isTrue
	offset_param=op["parameters"][2]
	assertoffset_param.get("has_default") isTrue

[bugraoz93]

Moving to draft to manage release better, please make it ready again when you think ready for maintainers review

[shivaam]

Moving to draft to manage release better, please make it ready again when you think ready for maintainers review
@bugraoz93!

Sorry I replied on the issue instead of the PR. argparse doesn't natively support this. Below I have listed some options.

---

I looked into supporting both positional and --flag styles for required params. Unfortunately argparse doesn't natively support this — a parameter is either positional (dag_id) or a flag (--dag-id), never both.

Why it's not straightforward:

The closest workaround is registering all params as --flags, adding a nargs='*' catch-all for bare values, and using parse_intermixed_args to map positionals into unfilled flag slots after parsing. More about this at the bottom of this comment. But this means we lose argparse's built-in validation, type checking, help formatting, and required=True for those params — we'd essentially reimplement positional argument handling ourselves on top of argparse.

Options:

1. Keep it positional for required params (current PR) — the simplest, most conventional choice. Required params are positional, optional params are flags. All argparse features work, clean help output.

2. Two-pass parsing — maintain two parsers per command: one with required params as positional, one with them as --flags required=True. Try positional first, fall back to flags. Users can use either style, just not mixed in the same invocation:

   # both work:
   airflowctl xcom add my_dag my_task my_key
   airflowctl xcom add --dag-id=my_dag --task-id=my_task --key=my_key
   # but not mixed:
   airflowctl xcom add my_dag --task-id=my_task my_key # would fail

   No argparse hacks needed (both parsers are standard), but doubles the parser setup and mixed usage isn't supported.

3. Cap positional count — first 1-2 required params (like dag_id) are positional, rest stay as --flag required=True. Common CLI pattern (like git checkout <branch> --force). No hacks, all argparse features work.

I'd recommend Option 1 for simplicity. Happy to go with whichever you prefer!

---

Details on the parse_intermixed_args workaround (and why I didn't recommend it)

The idea is to register all required params as --flags only (no argparse positionals), add a single nargs='*' argument to collect any bare values, then map those bare values into whichever flag slots the user didn't fill:

PARAM_ORDER= ['dag_id', 'task_id', 'key']
parser.add_argument('--dag-id', dest='dag_id', default=None)
parser.add_argument('--task-id', dest='task_id', default=None)
parser.add_argument('--key', default=None)
parser.add_argument('positionals', nargs='*')
args=parser.parse_intermixed_args(argv)
# map bare values into unfilled flag slots, left to rightunfilled= [dfordinPARAM_ORDERifgetattr(args, d) isNone]
fori, valinenumerate(args.positionals):
setattr(args, unfilled[i], val)

parse_intermixed_args (Python 3.7+) allows flags and bare values to be freely interspersed, so all of these would work:

airflowctl xcom add d t k # all positional
airflowctl xcom add --dag-id=d --task-id=t --key=k # all flags
airflowctl xcom add d --task-id=t k # mixed
airflowctl xcom add --task-id=t my_dag my_key # gap-filling

The problem is what we lose:

• Validation — can't use required=True on flags (argparse would reject positional-only usage), so we validate manually after gap-filling
• Type checking — type=int, choices=[...] etc. don't apply to the nargs='*' catch-all, so we type-check manually
• Help output — shows [positionals ...] instead of named params like DAG_ID TASK_ID, and no indication which flags are required vs optional
• argparse.REMAINDER and mutually exclusive groups with positionals — unsupported with parse_intermixed_args

This effectively means using argparse for flag parsing only and reimplementing positional handling on top of it.

[bugraoz93]

@shivaam, this will make things a bit complicated. The two-pass parsing is still complex. I would say before we finish adding the command to the parser, we can reprocess the args, maybe even in get_parser, which can also be responsible for parsing the command. We may need to update the building part a bit but it should be possible with reprocessing

Even though we are still around 0.x.x, still thinking about how we could implement this in the best position. As it will be a user-facing change, we need to create a devlist discussion and ask to only keep positional for required or maintain both.

My idea is adding both can save us from the entire discussion flow and we could be backwards compatible, even though this is not trivial with argparase in Python.

[bugraoz93]

Something like this
#65261

[shivaam]

Closing in favor of #65261 which takes a better approach — supporting both positional and flag styles simultaneously via argv pre-processing, so there's no breaking change for existing users. Thanks @bugraoz93 for the guidance!

[shivaam]

Something like this #65261

I was just thinking about this. Supporting both positional and flag styles for the same param isn't something most CLI libraries or popular tools do. We're adding maintenance burden, constraining future changes as all CLI libraries might not suppor this feature, and expanding the testing surface forever. Since airflowctl is still pre-1.0, feels like this is the natural window for a clean break rather than committing to maintaining a non-standard approach indefinitely.

Since the layer we are adding is light, either way works. Just wanted to share my thoughts.

[bugraoz93]

Thanks @shivaam for your patience and joining the discussion! We have some procedures to follow as a community. :) Reopened yours, feel free to rebase and if ready we can ship

[bugraoz93]

https://lists.apache.org/thread/m1qvcvow3l17ytv40vhslh40wn3rntrm

[shivaam]

Sounds good. I will rebase the project.

[shivaam]

Before this PR, airflowctl backfill list (no args) returned [] and exited 0, but that was a silent bug — the CLI passed dag_id=None, which the server coerced to an empty filter that matched no records. The command was never actually returning backfills.

[bugraoz93]

Looks good! Thanks for your patience and work @shivaam!

[bugraoz93]

Added the full test label to see the integration tests green. We can merge after seeing it green :)

[bugraoz93]

CI looks unrelated, ctl test looks good

[bugraoz93]

@shivaam Could you please rebase and resolve conflicts, we can merge afterwards

[shivaam]

@bugraoz93 Seems like there is another PR that already solved this problem PR #66768 — "airflowctl: make required CLI params positional, keep optional as --flag" by 1fanwang
I dont think we need this anymore.

[bugraoz93]

Sorry @shivaam! Totally missed that one. I will try to be faster next time 😅 It also has some release notes changes need manually handled while backporting and should be cleaned while generating

[shivaam]

Required non-boolean parameters in auto-generated CLI commands are now positional instead of flag-style, improving UX consistency with manually defined commands like dags pause.

Before:

airflowctl connections create --connection-id="test" --conn-type="mysql" --password=secret
airflowctl dags get --dag-id=example_bash_operator

After:

airflowctl connections create test mysql --password=secret
airflowctl dags get example_bash_operator

Changes

• Track which operation parameters have default values during AST parsing
• For primitive params, use the default info to decide positional vs flag
• For Pydantic model fields, use is_required() to decide positional vs flag
• Use _UNSET for arg_dest default to avoid argparse crash on positional args
• Skip the has_default metadata key at runtime when mapping CLI args to method params
• Updated unit tests and integration test commands to match new positional syntax
• Added 2 new tests for positional arg behavior and default detection
• Added newsfragment for the breaking change

Notes

• _create_arg stays a dumb pass-through builder — all positional/flag logic is in the callers via one-liner conditionals
• Booleans are never positional (they need --flag/--no-flag)
• No special-casing for specific operations
• Breaking change: existing --flag=value syntax for required params will no longer work. Since airflowctl is new and not yet stable, this should be acceptable.

Open question

Posted a clarifying question on the issue about whether all required params should be positional (e.g., xcom add has 5 positional args), or if we should limit it.

closes: #60142

---

Was generative AI tooling used to co-author this PR?

• Yes — Claude Code (Claude Opus 4.6)

Generated-by: Claude Code (Claude Opus 4.6) following the guidelines

[bugraoz93]

Thanks for the PR! I addressed your question. Could you please amend it to be both optional and positional, live side by side?
This way, we won't break anything while working, but rather add Airflow CLI positional arguments to improve parity and make migrations easier.

[bugraoz93]

This news fragment is for Airflow Core and not related to anything within airflowctl flow.
Since we are still in 0.x, significant changes are also expected at a certain level.
We are logging these things in the release notes

https://airflow.apache.org/docs/apache-airflow-ctl/stable/release_notes.html#significant-changes

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates auto-generated airflowctl commands so required non-boolean parameters are positional (UX aligned with manually-defined commands), and adjusts parsing/tests to support the breaking CLI syntax change.

Changes:

• Track whether operation parameters have defaults during AST parsing and use it to decide positional vs flag args for primitive params.
• Use Pydantic model_fields[].is_required() to decide positional vs flag args for datamodel fields (booleans remain flags).
• Update unit/integration tests and add a newsfragment documenting the breaking change.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py	Updates assertions for positional args; adds tests for positional behavior and default detection.
airflow-ctl/src/airflowctl/ctl/cli_config.py	Adds has_default metadata via AST parsing; generates positional args for required params; skips metadata during arg mapping and invocation.
airflow-ctl-tests/tests/airflowctl_tests/test_config_sensitive_masking.py	Updates integration test commands to new positional syntax.
airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py	Updates end-to-end command list to match new positional syntax across many commands.
airflow-core/newsfragments/64812.significant.rst	Documents the breaking CLI change for auto-generated commands.

[Copilot]

The parameters entries are now shaped as a dict with a dynamic key for the param name plus a reserved has_default key. This forces downstream code to iterate parameter.items() and special-case parameter_key == \"has_default\", which is brittle and harder to extend (and also makes collisions with a real param named has_default difficult to reason about). Consider switching to a stable structure like { \"name\": ..., \"type\": ..., \"has_default\": ... } for each parameter so consumers can access fields directly without item-iteration + skips.

Suggested change

	args.append({arg_name: arg_type, "has_default": has_default})
	args.append(
	{"name": arg_name, "type": arg_type, "has_default": has_default}
	)

[Copilot]

Related to the parameter-shape issue: the for parameter_key, parameter_type in parameter.items() loop plus if parameter_key == \"has_default\": continue is an avoidable control-flow hack that spreads across multiple call sites (also in _get_func). If you keep the current shape, a safer alternative is to store metadata under a dedicated nested key (e.g. {\"name\": ..., \"type\": ..., \"meta\": {\"has_default\": ...}}) or move metadata out of the per-parameter dict entirely so this loop can be simplified and cannot accidentally skip real parameters.

[Copilot]

This test writes a fixed filename (test_has_default.py) into the working directory, which can conflict under parallel test runs or when the CWD is not writable. Use pytest’s tmp_path/tmp_path_factory (or tempfile.NamedTemporaryFile) to create an isolated temporary file path, and open it with an explicit encoding (e.g. encoding=\"utf-8\") to match the production file-read behavior.

Suggested change

	deftest_has_default_detection_in_ast_parsing(self):
	"""Test that AST parsing correctly detects which params have defaults."""
	fromtextwrapimportdedent
	temp_file="test_has_default.py"
	withopen(temp_file, "w") asf:
	f.write(
	dedent("""
	classTestOperations(BaseOperations):
	defget(self, required_id: str) ->str|ServerResponseError:
	pass
	defsearch(self, query: str, limit: int=10, offset: int=0) ->str|ServerResponseError:
	pass
	""")
	)
	try:
	command_factory=CommandFactory(file_path=temp_file)
	foropincommand_factory.operations:
	ifop["name"] =="get":
	param=op["parameters"][0]
	assertparam.get("has_default") isFalse
	elifop["name"] =="search":
	query_param=op["parameters"][0]
	assertquery_param.get("has_default") isFalse
	limit_param=op["parameters"][1]
	assertlimit_param.get("has_default") isTrue
	offset_param=op["parameters"][2]
	assertoffset_param.get("has_default") isTrue
	finally:
	importos
	os.remove(temp_file)
	deftest_has_default_detection_in_ast_parsing(self, tmp_path):
	"""Test that AST parsing correctly detects which params have defaults."""
	fromtextwrapimportdedent
	temp_file=tmp_path/"test_has_default.py"
	temp_file.write_text(
	dedent("""
	classTestOperations(BaseOperations):
	defget(self, required_id: str) ->str|ServerResponseError:
	pass
	defsearch(self, query: str, limit: int=10, offset: int=0) ->str|ServerResponseError:
	pass
	"""),
	encoding="utf-8",
	)
	command_factory=CommandFactory(file_path=str(temp_file))
	foropincommand_factory.operations:
	ifop["name"] =="get":
	param=op["parameters"][0]
	assertparam.get("has_default") isFalse
	elifop["name"] =="search":
	query_param=op["parameters"][0]
	assertquery_param.get("has_default") isFalse
	limit_param=op["parameters"][1]
	assertlimit_param.get("has_default") isTrue
	offset_param=op["parameters"][2]
	assertoffset_param.get("has_default") isTrue

[bugraoz93]

Moving to draft to manage release better, please make it ready again when you think ready for maintainers review

[shivaam]

Moving to draft to manage release better, please make it ready again when you think ready for maintainers review
@bugraoz93!

Sorry I replied on the issue instead of the PR. argparse doesn't natively support this. Below I have listed some options.

---

I looked into supporting both positional and --flag styles for required params. Unfortunately argparse doesn't natively support this — a parameter is either positional (dag_id) or a flag (--dag-id), never both.

Why it's not straightforward:

The closest workaround is registering all params as --flags, adding a nargs='*' catch-all for bare values, and using parse_intermixed_args to map positionals into unfilled flag slots after parsing. More about this at the bottom of this comment. But this means we lose argparse's built-in validation, type checking, help formatting, and required=True for those params — we'd essentially reimplement positional argument handling ourselves on top of argparse.

Options:

1. Keep it positional for required params (current PR) — the simplest, most conventional choice. Required params are positional, optional params are flags. All argparse features work, clean help output.

2. Two-pass parsing — maintain two parsers per command: one with required params as positional, one with them as --flags required=True. Try positional first, fall back to flags. Users can use either style, just not mixed in the same invocation:

   # both work:
   airflowctl xcom add my_dag my_task my_key
   airflowctl xcom add --dag-id=my_dag --task-id=my_task --key=my_key
   # but not mixed:
   airflowctl xcom add my_dag --task-id=my_task my_key # would fail

   No argparse hacks needed (both parsers are standard), but doubles the parser setup and mixed usage isn't supported.

3. Cap positional count — first 1-2 required params (like dag_id) are positional, rest stay as --flag required=True. Common CLI pattern (like git checkout <branch> --force). No hacks, all argparse features work.

I'd recommend Option 1 for simplicity. Happy to go with whichever you prefer!

---

Details on the parse_intermixed_args workaround (and why I didn't recommend it)

The idea is to register all required params as --flags only (no argparse positionals), add a single nargs='*' argument to collect any bare values, then map those bare values into whichever flag slots the user didn't fill:

PARAM_ORDER= ['dag_id', 'task_id', 'key']
parser.add_argument('--dag-id', dest='dag_id', default=None)
parser.add_argument('--task-id', dest='task_id', default=None)
parser.add_argument('--key', default=None)
parser.add_argument('positionals', nargs='*')
args=parser.parse_intermixed_args(argv)
# map bare values into unfilled flag slots, left to rightunfilled= [dfordinPARAM_ORDERifgetattr(args, d) isNone]
fori, valinenumerate(args.positionals):
setattr(args, unfilled[i], val)

parse_intermixed_args (Python 3.7+) allows flags and bare values to be freely interspersed, so all of these would work:

airflowctl xcom add d t k # all positional
airflowctl xcom add --dag-id=d --task-id=t --key=k # all flags
airflowctl xcom add d --task-id=t k # mixed
airflowctl xcom add --task-id=t my_dag my_key # gap-filling

The problem is what we lose:

• Validation — can't use required=True on flags (argparse would reject positional-only usage), so we validate manually after gap-filling
• Type checking — type=int, choices=[...] etc. don't apply to the nargs='*' catch-all, so we type-check manually
• Help output — shows [positionals ...] instead of named params like DAG_ID TASK_ID, and no indication which flags are required vs optional
• argparse.REMAINDER and mutually exclusive groups with positionals — unsupported with parse_intermixed_args

This effectively means using argparse for flag parsing only and reimplementing positional handling on top of it.

[bugraoz93]

@shivaam, this will make things a bit complicated. The two-pass parsing is still complex. I would say before we finish adding the command to the parser, we can reprocess the args, maybe even in get_parser, which can also be responsible for parsing the command. We may need to update the building part a bit but it should be possible with reprocessing

Even though we are still around 0.x.x, still thinking about how we could implement this in the best position. As it will be a user-facing change, we need to create a devlist discussion and ask to only keep positional for required or maintain both.

My idea is adding both can save us from the entire discussion flow and we could be backwards compatible, even though this is not trivial with argparase in Python.

[bugraoz93]

Something like this
#65261

[shivaam]

Closing in favor of #65261 which takes a better approach — supporting both positional and flag styles simultaneously via argv pre-processing, so there's no breaking change for existing users. Thanks @bugraoz93 for the guidance!

[shivaam]

Something like this #65261

I was just thinking about this. Supporting both positional and flag styles for the same param isn't something most CLI libraries or popular tools do. We're adding maintenance burden, constraining future changes as all CLI libraries might not suppor this feature, and expanding the testing surface forever. Since airflowctl is still pre-1.0, feels like this is the natural window for a clean break rather than committing to maintaining a non-standard approach indefinitely.

Since the layer we are adding is light, either way works. Just wanted to share my thoughts.

[bugraoz93]

Thanks @shivaam for your patience and joining the discussion! We have some procedures to follow as a community. :) Reopened yours, feel free to rebase and if ready we can ship

[bugraoz93]

https://lists.apache.org/thread/m1qvcvow3l17ytv40vhslh40wn3rntrm

[shivaam]

Sounds good. I will rebase the project.

[shivaam]

Before this PR, airflowctl backfill list (no args) returned [] and exited 0, but that was a silent bug — the CLI passed dag_id=None, which the server coerced to an empty filter that matched no records. The command was never actually returning backfills.

[bugraoz93]

Looks good! Thanks for your patience and work @shivaam!

[bugraoz93]

Added the full test label to see the integration tests green. We can merge after seeing it green :)

[bugraoz93]

CI looks unrelated, ctl test looks good

[bugraoz93]

@shivaam Could you please rebase and resolve conflicts, we can merge afterwards

[shivaam]

@bugraoz93 Seems like there is another PR that already solved this problem PR #66768 — "airflowctl: make required CLI params positional, keep optional as --flag" by 1fanwang
I dont think we need this anymore.

[bugraoz93]

Sorry @shivaam! Totally missed that one. I will try to be faster next time 😅 It also has some release notes changes need manually handled while backporting and should be cleaned while generating