Fury0508 commented on PR #60193:
URL: https://github.com/apache/airflow/pull/60193#issuecomment-3726388286
@bugraoz93 I've addressed all the feedback:
**Changes made:**
1. Removed the `# NEW PARAMETER` comments and unnecessary explanatory
comments
2. Implemented the one-liner conditional approach as you suggested:
```python
arg_flags = (sanitized_field,) if is_required and not is_bool else ("--" +
sanitized_field,)
```
3. Simplified `_create_arg` to detect positional vs optional by checking if
flags start with `-`
4. Added unit test `test_positional_args`
5. Updated `test_args_create` fixture to expect positional args for required
fields
**What this solves:**
This implementation makes required non-boolean parameters (like
`connection_id`, `dag_id`, `from_date`) positional arguments without the `--`
prefix, while optional parameters keep the `--` prefix. This improves CLI UX by
making it clearer which parameters are mandatory.
**Example:**
```bash
# Before: All params had --
airflowctl connections create --connection-id test --conn-type http
# After: Required params are positional
airflowctl connections create test http [--optional-params]
```
The logic is now centralised in one place (the conditional when calling
`_create_arg`) rather than split between caller and callee, making it more
maintainable as you suggested.
Ready for review!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
