Rename `db upgrade` to `db migrate` and add `connections create-default-connections` by Adaverse · Pull Request #32810 · apache/airflow

Adaverse · 2023-07-24T18:46:58Z

Addresses: #32420
Related: #32048

Renamed db upgrade with ~~db sync~~ db migrate and deprecated the former
Deprecated db init and added connections create-default-connections to create the default connection
Added test for the above change
Document updation ongoing

This PR is part of PRs where we target to move the process of configuring default connections out of core Airflow to there respective providers.

^ 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 newsfragments.

Adaverse · 2023-07-24T18:47:29Z

cc - @potiuk @eladkal

potiuk · 2023-07-25T21:31:30Z

Two things:

I would add a "significant" newsfragment describing it. I also think there was a way
help=argparse.SUPPRESS I think is the way to hide the option from --help output and also not to show it in our documentaiton - which is basically what we want. old users who are used to it will stil be able to use it, new users won't because they won't discover it.

Adaverse · 2023-07-27T21:32:26Z

I also think there was a way help=argparse.SUPPRESS
I think is the way to hide the option from --help output and also not to show it in our documentaiton - which is basically what we want. old users who are used to it will stil be able to use it, new users won't because they won't discover it.

Yes we can hide arguments using argparse.SUPPRESS but it does not work when applied on sub_parser. It can be hidden by omitting help parameter from the corresponding sub commands as explained here. But this way it appears in the doc with text undocumented below the command.

I have added a constant that holds the subcommand that needs to be hidden.

uranusjr · 2023-07-28T05:31:59Z

Was there a discussion to use migrate instead? (Not that I object to migrate, just curious about the sudden change.)

Adaverse · 2023-07-28T05:53:23Z

Was there a discussion to use migrate instead? (Not that I object to migrate, just curious about the sudden change.)

Have created an announcement in the dev channel with context asking what everybody thinks - https://lists.apache.org/thread/3njd9l8fpcshnkk61t74orntp13w4vjo

The above change is not yet final in any way. Depends on collective agreement.

Do I also need to cc some other channels?

uranusjr · 2023-07-28T05:57:06Z

No this is good, I just wondered if it’s based on a decision made somewhere I’m not aware of.

potiuk · 2023-07-28T16:13:36Z

        func=lazy_load_command("airflow.cli.commands.db_command.drop_archived"),
        args=(ARG_DB_TABLES, ARG_YES),
    ),
+    ActionCommand(


Should we already move it to the connections command in this PR?

Yes, on it!

I hope it is fine now to replace every instance of db init and db upgrade with db migrate in the doc?

There are few places where initdb from airflow.utils.db is being called that creates default connection ex -

airflow/airflow/utils/db.py

Lines 1669 to 1670 in f91c003

if not skip_init:

initdb(session=session)

airflow/airflow/utils/db.py

Lines 845 to 848 in f91c003

if len(db_heads) < 1:

db_command = initdb

command_name = "init"

verb = "initialize"

Shall we replace these now with upgradedb or maybe in later commits when we would have added the desired functionality to create_default_connections?

I hope it is fine now to replace every instance of db init and db upgrade with db migrate in the doc?

yes

Shall we replace these now with upgradedb or maybe in later commits when we would have added the desired functionality to create_default_connections?

should be migrate in both cases I think. That was the intention in both cases, I think

should be migrate in both cases I think.

Currently, we have just renamed the command and the function names in db_commands.py. I think its best to take the renaming of airflow.utils.db.upgradedb to be taken in another PR since it is used at lots of other places and quite some refactoring is required for above change.

Co-authored-by: Jed Cunningham <[email protected]>

…eployment.rst Co-authored-by: Jed Cunningham <[email protected]>

Co-authored-by: Jed Cunningham <[email protected]>

In apache#32810, "airflow db migrate" command has been added and it is used by `start-airflow` command. There were a few edge cases not covered and this PR completes it. * We can move the "database/load_default_connections" configuration to a new "deprecated" section. This is the first time we remove the option completely as it lost its meaning, but we likely still want to explain that the option was there and what it does when deprecated "db init" command is used. It has no meaning when you use "airflow db migrate" or when you run the new "airflow connections create-default-connections" commands. So we can now remove it completely from configuraiton. It will still work in the "airflow db init" which is deprecated, as long as we provide an explicit fallback. Also if someone had it defined in their config or env variable, it will continue to work even if it is not defined. * We need to explain the change in a significant newsfragment. * The ``start-airflow`` command supports creating default connections with ``--load-default-connections`` flag. This was lost after the change so this PR brings it back by running the new "airflow connections create-default-connections" command if the flag is used. * The `start-airflow` breeze command can be used to start older versions of airflow - with ``--use-airflow-version" - those that do not support `airflow db migrate` command. In this case the old behaviour is used with setting the "AIRFLOW__DATABASE__LOAD_DEFAULT_CONNECTIONS" based on the flag passed and running "airflow db init" instead.

In #32810, "airflow db migrate" command has been added and it is used by `start-airflow` command. There were a few edge cases not covered and this PR completes it. * We can move the "database/load_default_connections" configuration to a new "deprecated" section. This is the first time we remove the option completely as it lost its meaning, but we likely still want to explain that the option was there and what it does when deprecated "db init" command is used. It has no meaning when you use "airflow db migrate" or when you run the new "airflow connections create-default-connections" commands. So we can now remove it completely from configuraiton. It will still work in the "airflow db init" which is deprecated, as long as we provide an explicit fallback. Also if someone had it defined in their config or env variable, it will continue to work even if it is not defined. * We need to explain the change in a significant newsfragment. * The ``start-airflow`` command supports creating default connections with ``--load-default-connections`` flag. This was lost after the change so this PR brings it back by running the new "airflow connections create-default-connections" command if the flag is used. * The `start-airflow` breeze command can be used to start older versions of airflow - with ``--use-airflow-version" - those that do not support `airflow db migrate` command. In this case the old behaviour is used with setting the "AIRFLOW__DATABASE__LOAD_DEFAULT_CONNECTIONS" based on the flag passed and running "airflow db init" instead.

In #32810, "airflow db migrate" command has been added and it is used by `start-airflow` command. There were a few edge cases not covered and this PR completes it. * We can move the "database/load_default_connections" configuration to a new "deprecated" section. This is the first time we remove the option completely as it lost its meaning, but we likely still want to explain that the option was there and what it does when deprecated "db init" command is used. It has no meaning when you use "airflow db migrate" or when you run the new "airflow connections create-default-connections" commands. So we can now remove it completely from configuraiton. It will still work in the "airflow db init" which is deprecated, as long as we provide an explicit fallback. Also if someone had it defined in their config or env variable, it will continue to work even if it is not defined. * We need to explain the change in a significant newsfragment. * The ``start-airflow`` command supports creating default connections with ``--load-default-connections`` flag. This was lost after the change so this PR brings it back by running the new "airflow connections create-default-connections" command if the flag is used. * The `start-airflow` breeze command can be used to start older versions of airflow - with ``--use-airflow-version" - those that do not support `airflow db migrate` command. In this case the old behaviour is used with setting the "AIRFLOW__DATABASE__LOAD_DEFAULT_CONNECTIONS" based on the flag passed and running "airflow db init" instead. (cherry picked from commit b672ba4)

adaverse added 3 commits July 24, 2023 23:41

db sync and create-default-connections added

e448818

fixed static checks

9f6a32c

update

84b1b09

boring-cyborg Bot added the area:CLI label Jul 24, 2023

Adaverse changed the title ~~Renames db upgrade with db sync and adds create-default-connections~~ Rename db upgrade with db sync and add create-default-connections Jul 24, 2023

Adaverse changed the title ~~Rename db upgrade with db sync and add create-default-connections~~ Rename db upgrade to db sync and add create-default-connections Jul 25, 2023

hide init and upgrade in -h

0fcec86

eladkal added this to the Airflow 2.7.0 milestone Jul 28, 2023

potiuk reviewed Jul 28, 2023

View reviewed changes

move create-default-connection from db to connections

a4629fe

Adaverse changed the title ~~Rename db upgrade to db sync and add create-default-connections~~ Rename db upgrade to db migrate and add connections create-default-connections Jul 28, 2023

potiuk approved these changes Jul 28, 2023

View reviewed changes

Adaverse marked this pull request as draft July 28, 2023 22:04

updated docs and usages

caff59e

Adaverse marked this pull request as ready for review July 28, 2023 22:21

Adaverse requested review from XD-DENG, ashb, dstandish, jedcunningham and kaxil as code owners July 28, 2023 22:21

Adaverse commented Jul 28, 2023

View reviewed changes

Comment thread docs/docker-stack/entrypoint.rst

Comment thread scripts/docker/entrypoint_prod.sh

Adaverse requested a review from potiuk July 28, 2023 22:24

adaverse added 2 commits July 29, 2023 03:54

db sync and create-default-connections added

0c3207a

fixed static checks

6946687

Adaverse and others added 9 commits July 31, 2023 22:20

Update airflow/cli/commands/db_command.py

5f319cb

Co-authored-by: Jed Cunningham <[email protected]>

Update airflow/cli/commands/db_command.py

ac4b7a7

Co-authored-by: Jed Cunningham <[email protected]>

Update airflow/models/crypto.py

1c6bad9

Co-authored-by: Jed Cunningham <[email protected]>

Update docs/apache-airflow/administration-and-deployment/production-d…

528e36a

…eployment.rst Co-authored-by: Jed Cunningham <[email protected]>

Update docs/apache-airflow/installation/setting-up-the-database.rst

08629d9

Co-authored-by: Jed Cunningham <[email protected]>

Update docs/apache-airflow/installation/setting-up-the-database.rst

d5c8c46

Co-authored-by: Jed Cunningham <[email protected]>

Update docs/apache-airflow/installation/upgrading.rst

afd97b3

Co-authored-by: Jed Cunningham <[email protected]>

Update docs/apache-airflow/installation/upgrading.rst

be6d072

Co-authored-by: Jed Cunningham <[email protected]>

review changes

35b760f

Adaverse requested a review from jedcunningham July 31, 2023 18:38

Adaverse and others added 2 commits August 1, 2023 00:12

Update airflow/cli/commands/db_command.py

69e9857

Co-authored-by: Jed Cunningham <[email protected]>

tests fixed

caaf024

jedcunningham reviewed Jul 31, 2023

View reviewed changes

Comment thread airflow/cli/cli_parser.py

jedcunningham approved these changes Jul 31, 2023

View reviewed changes

potiuk merged commit 1e20ef2 into apache:main Jul 31, 2023

ephraimbuddy added the type:improvement Changelog: Improvements label Aug 2, 2023

potiuk mentioned this pull request Aug 5, 2023

Fix edge cases of "migrate/create-default-connections" #33136

Merged

	if len(db_heads) < 1:
	db_command = initdb
	command_name = "init"
	verb = "initialize"

Uh oh!

Conversation

Adaverse commented Jul 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Adaverse commented Jul 24, 2023

Uh oh!

potiuk commented Jul 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Adaverse commented Jul 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

uranusjr commented Jul 28, 2023

Uh oh!

Adaverse commented Jul 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

uranusjr commented Jul 28, 2023

Uh oh!

potiuk Jul 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Adaverse Jul 28, 2023

Choose a reason for hiding this comment

Uh oh!

Adaverse Jul 28, 2023

Choose a reason for hiding this comment

Uh oh!

Adaverse Jul 28, 2023

Choose a reason for hiding this comment

Uh oh!

potiuk Jul 28, 2023

Choose a reason for hiding this comment

Uh oh!

Adaverse Jul 28, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Adaverse commented Jul 24, 2023 •

edited

Loading

potiuk commented Jul 25, 2023 •

edited

Loading

Adaverse commented Jul 27, 2023 •

edited

Loading

Adaverse commented Jul 28, 2023 •

edited

Loading

potiuk Jul 28, 2023 •

edited

Loading