Skip to content

Conversation

o-nikolas
Copy link
Contributor

CLI commands for Create, List, Delete of teams. These are delivered as CLI commands instead of API because they are administrator only (similar to airflow db reset). These are not commands that DAG authors should be regularly running (therefore no APIs implemented)


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

CLI commands for Create, List, Delete of teams. These are delivered as
CLI commands instead of API because they are administrator only (similar
to `airflow db reset`). These are not commands that DAG authors should
be regularly running (therefore no APIs implemented)
@@ -305,6 +305,8 @@ def string_lower_type(val):
ARG_LOCAL = Arg(("-l", "--local"), help="Run the task using the LocalExecutor", action="store_true")
ARG_POOL = Arg(("--pool",), "Resource pool to use")

# teams
ARG_TEAM_NAME = Arg(("name",), help="Team name")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if you add required=True? Then you could remove _extract_team_name

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you will still need it because it is possible to pass things like empty string as the argument. But I still like making this required, so I'll make that change either way!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I just double checked and positional arguments are already required. But the input sanitation is still needed (for inputs like empty string).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants