Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Deprecate "paritydb-experimental" CLI in favour or "paritydb" #10975

Merged
merged 2 commits into from
Mar 4, 2022
Merged

Deprecate "paritydb-experimental" CLI in favour or "paritydb" #10975

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b838ff2
Deprecate paritydb-experimental
arkpar Mar 3, 2022
b642443
Updated comment
arkpar Mar 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions client/cli/src/arg_enums.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,10 @@ pub enum Database {
/// ParityDb. <https://github.com/paritytech/parity-db/>
ParityDb,
/// Detect whether there is an existing database. Use it, if there is, if not, create new
/// instance of paritydb
/// instance of ParityDb
Auto,
/// ParityDb. <https://github.com/paritytech/parity-db/>
ParityDbDeprecated,
}

impl std::str::FromStr for Database {
Expand All @@ -212,6 +214,8 @@ impl std::str::FromStr for Database {
if s.eq_ignore_ascii_case("rocksdb") {
Ok(Self::RocksDb)
} else if s.eq_ignore_ascii_case("paritydb-experimental") {
Ok(Self::ParityDbDeprecated)
Copy link
Member Author

Choose a reason for hiding this comment

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

This function gets called multiple times, so can't just print a warning here.

Copy link
Member

Choose a reason for hiding this comment

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

We could only log it once using OnceCell for example

Copy link
Member Author

Choose a reason for hiding this comment

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

Can't say I like this idea. Introducing global variable and yet another dependency for a trivial thing.

} else if s.eq_ignore_ascii_case("paritydb") {
Ok(Self::ParityDb)
} else if s.eq_ignore_ascii_case("auto") {
Ok(Self::Auto)
Expand All @@ -224,7 +228,7 @@ impl std::str::FromStr for Database {
impl Database {
/// Returns all the variants of this enum to be shown in the cli.
pub fn variants() -> &'static [&'static str] {
&["rocksdb", "paritydb-experimental", "auto"]
&["rocksdb", "paritydb", "paritydb-experimental", "auto"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I would just remove paritydb-experimental from the list (not sure if it is working).

Copy link
Member Author

Choose a reason for hiding this comment

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

It does not work. Clap produces an error if an unknown variant is used.

}
}

Expand Down
7 changes: 7 additions & 0 deletions client/cli/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,13 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
Ok(match database {
Database::RocksDb => DatabaseSource::RocksDb { path: rocksdb_path, cache_size },
Database::ParityDb => DatabaseSource::ParityDb { path: paritydb_path },
Database::ParityDbDeprecated => {
eprintln!(
Copy link
Member Author

Choose a reason for hiding this comment

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

Using log here is problematic because it is not initialised yet.

"WARNING: \"paritydb-experimental\" database setting is deprecated and will be removed in future releases. \
Please update your setup to use the new value: \"paritydb\"."
);
DatabaseSource::ParityDb { path: paritydb_path }
},
Database::Auto => DatabaseSource::Auto { paritydb_path, rocksdb_path, cache_size },
})
}
Expand Down