From 3ea91b8d42a7ceb6e4c6484a4e0982d50fa7cc52 Mon Sep 17 00:00:00 2001 From: Patrick Sullivan Date: Tue, 10 Dec 2024 21:02:24 -0600 Subject: [PATCH] chore: mark old migrations as deprecated for v0.14.0 release (#5308) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #5307 ### Summary From the issue description: > We would like to deprecate various old migrations. If these migrations are marked as deprecated in v0.14.0, then they will be officially removed from the app initialization logic in v0.17.0. > > The ramification of this is that users will not be able to upgrade directly from versions < 0.14.0 to versions >= v0.17.0. Instead, for example, a user on v0.13.0 would first need to upgrade to v0.14.0, and then from v0.14.0 they could upgrade to v0.17.0. > > Removing these migrations will reduce the risk that the behavior of some of these migrations, particularly those that reference data structures or logic in other parts of the application code base, accidentally changes in the future resulting in inconsistent migration behavior. ### Technical details This PR just marks certain sections of the code as deprecated. This deprecated logic will be removed some time after v0.17.0. ### 📣 Note to reviewers Please comment on any additional migrations that we would like to deprecate for v0.14.0 and I can include it in this PR. Or feel free to commit to this branch to mark additional migrations as deprecated. If you commit to this branch please use the `#[deprecated]` attribute if possible or include the word `deprecated` in a comment on the code; I plan on searching the source code for the word `"deprecated"` after v0.17.0 to remove the deprecated code, so this would help with that. ### Todo in v0.14.0 release We will want to include information in our official v0.14.0 release notes about... - The deprecation of migration/upgrade logic - The incompatibility of direct upgrades from < v0.14.0 to >= v0.17.0 - Instructions for upgrading from < v0.14.0 to >= v0.17.0 - Users will first need to upgrade to some version >= v0.14.0 and < v0.17.0. And then they will be able to upgrade to v0.17.0 ## Summary by CodeRabbit - **New Features** - Enhanced error handling for the `delete-parquet` command, providing clearer feedback on deletion success or failure. - Added deprecation warnings for several commands and functions, indicating future removals. - **Bug Fixes** - Improved feedback for unsupported components in the `view` command. - **Documentation** - Added comments clarifying the deprecation status of certain migration logic and functions. --- src/cli/basic/cli.rs | 1 + src/common/migration/dashboards.rs | 9 +++++++++ src/common/migration/mod.rs | 4 ++++ src/common/migration/schema.rs | 1 + src/main.rs | 1 + 5 files changed, 16 insertions(+) diff --git a/src/cli/basic/cli.rs b/src/cli/basic/cli.rs index edec397bbf..cffec1c9bf 100644 --- a/src/cli/basic/cli.rs +++ b/src/cli/basic/cli.rs @@ -271,6 +271,7 @@ pub async fn cli() -> Result { } "migrate-schemas" => { println!("Running schema migration to row per schema version"); + #[allow(deprecated)] migration::schema::run().await? } _ => { diff --git a/src/common/migration/dashboards.rs b/src/common/migration/dashboards.rs index 5f58f68c4b..fe31289c3d 100644 --- a/src/common/migration/dashboards.rs +++ b/src/common/migration/dashboards.rs @@ -88,6 +88,10 @@ async fn add_distinct_from_dashboard( pub async fn run() -> Result<(), anyhow::Error> { // load dashboards list let db = infra_db::get_db().await; + + // The following for-loop, which assigns all dashboards in the meta table to + // a default folder, is deprecated as of v0.14.0 and will be removed in + // v0.17.0. let db_key = "/dashboard/".to_string(); let data = db.list(&db_key).await?; for (key, val) in data { @@ -108,6 +112,11 @@ pub async fn run() -> Result<(), anyhow::Error> { } } + // The logic that follows is dependent on the dashboards table created in + // the SeaORM migration list. Unlike the logic above, it will not be + // deprecated in v0.14.0, though we could probably move it into its own + // migration script in the SeaORM migration list so that it is tracked by + // SeaORM. if distinct_values::len().await? > 0 { log::info!("dashboard distinct values migration already done."); } else { diff --git a/src/common/migration/mod.rs b/src/common/migration/mod.rs index 25fa742d5d..0c35c9458a 100644 --- a/src/common/migration/mod.rs +++ b/src/common/migration/mod.rs @@ -38,6 +38,7 @@ pub async fn check_upgrade(old_ver: &str, new_ver: &str) -> Result<(), anyhow::E log::info!("Upgrading from {} to {}", old_ver, new_ver); let v093 = Version::from("v0.9.3").unwrap(); if old_ver < v093 { + #[allow(deprecated)] upgrade_092_093().await?; } @@ -49,8 +50,10 @@ pub async fn check_upgrade(old_ver: &str, new_ver: &str) -> Result<(), anyhow::E Ok(()) } +#[deprecated(since = "0.14.0", note = "will be removed in 0.17.0")] async fn upgrade_092_093() -> Result<(), anyhow::Error> { // migration schema + #[allow(deprecated)] schema::run().await?; Ok(()) @@ -63,6 +66,7 @@ async fn upgrade_130_131() -> Result<(), anyhow::Error> { Ok(()) } +#[deprecated(since = "0.14.0", note = "will be removed in 0.17.0")] pub async fn upgrade_resource_names() -> Result<(), anyhow::Error> { // The below migration requires ofga init ready, but on Router node, // we don't initialize ofga, hence the migration should not run on router diff --git a/src/common/migration/schema.rs b/src/common/migration/schema.rs index d0792e659d..2729769037 100644 --- a/src/common/migration/schema.rs +++ b/src/common/migration/schema.rs @@ -47,6 +47,7 @@ use crate::{ const SCHEMA_MIGRATION_KEY: &str = "/migration/schema_versions/status"; const META_MIGRATION_VERSION_KEY: &str = "/migration/meta/version"; +#[deprecated(since = "0.14.0", note = "will be removed in 0.17.0")] pub async fn run() -> Result<(), anyhow::Error> { match upgrade_schema_row_per_version().await { std::result::Result::Ok(true) => { diff --git a/src/main.rs b/src/main.rs index 07559cda96..3f3f33abf3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -227,6 +227,7 @@ async fn main() -> Result<(), anyhow::Error> { panic!("migrate dashboards failed: {}", e); } + #[allow(deprecated)] migration::upgrade_resource_names() .await .expect("migrate resource names into supported ofga format failed");