Add migration guide for CLI commands #10078
Conversation
turbaszek
added
the
area:upgrade
docs/index.rst
Outdated
| @@ -108,6 +108,7 @@ Content | |||
|
|
|||
| Operators and hooks <operators-and-hooks-ref> | |||
| CLI <cli-ref> | |||
| CLI migration guide <cli-migration-guide> | |||
There was a problem hiding this comment.
In the next PR, maybe you could create a dedicated "Airflow 2.0 Migration Guides" section and have all the migration under that, WDYT??
There was a problem hiding this comment.
Nice. I will do that.
docs/cli-migration-guide.rst
Outdated
|
|
||
| * - ``airflow flower`` | ||
| - ``airflow celery flower`` | ||
| - ``celery`` |
There was a problem hiding this comment.
Can you add information about flags?
https://github.com/apache/airflow/blob/master/UPDATING.md#simplification-of-cli-commands
docs/cli-migration-guide.rst
Outdated
| - ``airflow pools`` | ||
| - ``pools`` | ||
|
|
||
| * - ``airflow create_user`` |
There was a problem hiding this comment.
Can you also update this section in UPDATING.md?
https://github.com/apache/airflow/blob/master/UPDATING.md#cli-changes
Now we have conflicting information because there is no users --create command in any release.
There was a problem hiding this comment.
Does it seem like the flags are not yet effective? When I try the flags it doesn't work
There was a problem hiding this comment.
Yes. In that case, we should update this entry.
|
Can you move this content to UPDAATING.md? I think that this way it will make it easier to find this guide. We'll also be able to update it post-release if we discover new breaking changes. |
UPDATING.md
Outdated
| ``` | ||
|
|
||
| To remove a user from a role: | ||
| ```bash | ||
| airflow users --remove-role --username jondoe --role Public | ||
| airflow users remove-role --username jondoe --role Public | ||
| ``` | ||
|
|
||
| #### CLI reorganization |
There was a problem hiding this comment.
I have the impression that we can combine these sections with the next.
There was a problem hiding this comment.
Ok. I will do that now
UPDATING.md
Outdated
| together as subcommands. The `airflow list_dags` command is now `airflow | ||
| dags list`, `airflow pause` is `airflow dags pause`, `airflow config` is `airflow config list`, etc. | ||
| The `airflow list_dags` command is now `airflow dags list`, `airflow pause` is `airflow dags pause`, | ||
| `airflow config` is `airflow config list`, etc. |
There was a problem hiding this comment.
Can you add explicit text to highlight that in Airflow 1.10 and 2.0 there is an airflow config command but there is a difference in behavior. In Airflow 1.10, it prints all config options. In Airflow 2.0, it's a command group.
UPDATING.md
Outdated
| @@ -78,50 +78,76 @@ This section describes the changes that have been made, and what you need to do | |||
|
|
|||
| #### Simplification of CLI commands | |||
|
|
|||
| The ability to manipulate users from the command line has been changed. 'airflow create_user' and 'airflow delete_user' and 'airflow list_users' has been grouped to a single command `airflow users` with optional flags `--create`, `--list` and `--delete`. | |||
| The ability to manipulate users from the command line has been changed. ``airflow create_user``, ``airflow delete_user`` | |||
| and ``airflow list_users`` has been grouped to a single command `airflow users` with optional flags `create`, `list` and `delete`. | |||
There was a problem hiding this comment.
In my opinion, these sections can be combined to the next, but examples can be added below the table.
| | ``airflow pool`` | ``airflow pools`` | ``pools`` | | ||
| | ``airflow create_user`` | ``airflow users create`` | ``users`` | | ||
| | ``airflow delete_user`` | ``airflow users delete`` | ``users`` | | ||
| | ``airflow list_users`` | ``airflow users list`` | ``users`` | |
There was a problem hiding this comment.
I still think we should add "legacy" commands in Airflow CLI to print what is the new command and exit with error. This is a better way of "documenting" it than adding UPDATING.md , Most people will not read the UPDATING.md so we should make it as easy and straightforward for the users to get informed about the new command when they run the old one.
Since we do not have an overlap between the old and new commands it should be rather easy to do (now that we have a table with those commands it should be rather easy.
Do you plan to do it within this PR @ephraimbuddy @mik-laj? Or as a separate, follow-up PR?
There was a problem hiding this comment.
My mind is that regardless of the message available to the user as an in-program message, we still need to provide documentation that will summarize the changes. I can't imagine that the user will have to run each command manually to find out what the new name is.
Similarly, the warning about the change of the import path in the module code is not enough. My belief is that users will want to read the documents to know what to do. Of course, not everyone will do it, but some will want access to this type of documentation.
I think this ticket focuses only on documentation for now. In the next step, we can also add hints as messages in CLI.
Since we do not have an overlap between the old and new commands it should be rather easy to do (now that we have a table with those commands it should be rather easy.
We have one overlapping command - airflow config.
There was a problem hiding this comment.
Could you please create a separate ticket and describe your idea?
There was a problem hiding this comment.
We definitely need to have a description in one place. This is absolutely needed and I never denied it.
But my point is that it will be far more useful for the users if they do not have to look it up in UPDATING.md if they run an old command. UPDATING.md will be HUGE and I just try to think as a user using airflow and being used to run familiar commands.
Giving them suggestion what they can use instead is much better solution than asking them to look it up somewhere (UPDATING.md will not be readily available after you install Airflow - you will have to look it up).
I understand then that it has not been planned in this ticket right?
There was a problem hiding this comment.
I think this is just a documentation update for now, but it is a step that will allow us to work on your idea as well.
There was a problem hiding this comment.
Still it needs a rebase @ephraimbuddy :)
There was a problem hiding this comment.
I will be happy to implement the ticket when raised.
Closes: #9952
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 UPDATING.md.