refactor: add '&mut Plugins' argument in plugins setup api and remove…

… unnecessary mut Signed-off-by: zyy17 <zyylsxm@gmail.com>
GreptimeTeam · Jul 19, 2024 · 9b35dff · 9b35dff
1 parent 0b13ac6
commit 9b35dff
Show file tree

Hide file tree

Showing 7 changed files with 55 additions and 31 deletions.
diff --git a/src/cmd/src/datanode.rs b/src/cmd/src/datanode.rs
@@ -18,6 +18,7 @@ use std::time::Duration;
 use async_trait::async_trait;
 use catalog::kvbackend::MetaKvBackend;
 use clap::Parser;
+use common_base::Plugins;
 use common_config::Configurable;
 use common_telemetry::logging::TracingOptions;
 use common_telemetry::{info, warn};
@@ -271,8 +272,9 @@ impl StartCommand {
         info!("Datanode start command: {:#?}", self);
         info!("Datanode options: {:#?}", opts);
 
-        let mut opts = opts.component;
-        let plugins = plugins::setup_datanode_plugins(&mut opts)
+        let opts = opts.component;
+        let mut plugins = Plugins::new();
+        plugins::setup_datanode_plugins(&mut plugins, &opts)
             .await
             .context(StartDatanodeSnafu)?;
 

diff --git a/src/cmd/src/frontend.rs b/src/cmd/src/frontend.rs
@@ -20,6 +20,7 @@ use cache::{build_fundamental_cache_registry, with_default_composite_cache_regis
 use catalog::kvbackend::{CachedMetaKvBackendBuilder, KvBackendCatalogManager, MetaKvBackend};
 use clap::Parser;
 use client::client_manager::NodeClients;
+use common_base::Plugins;
 use common_config::Configurable;
 use common_grpc::channel_manager::ChannelConfig;
 use common_meta::cache::{CacheRegistryBuilder, LayeredCacheRegistryBuilder};
@@ -264,9 +265,9 @@ impl StartCommand {
         info!("Frontend start command: {:#?}", self);
         info!("Frontend options: {:#?}", opts);
 
-        let mut opts = opts.component;
-        #[allow(clippy::unnecessary_mut_passed)]
-        let plugins = plugins::setup_frontend_plugins(&mut opts)
+        let opts = opts.component;
+        let mut plugins = Plugins::new();
+        plugins::setup_frontend_plugins(&mut plugins, &opts)
             .await
             .context(StartFrontendSnafu)?;
 
@@ -458,7 +459,7 @@ mod tests {
 
     #[tokio::test]
     async fn test_try_from_start_command_to_anymap() {
-        let mut fe_opts = frontend::frontend::FrontendOptions {
+        let fe_opts = frontend::frontend::FrontendOptions {
             http: HttpOptions {
                 disable_dashboard: false,
                 ..Default::default()
@@ -467,8 +468,10 @@ mod tests {
             ..Default::default()
         };
 
-        #[allow(clippy::unnecessary_mut_passed)]
-        let plugins = plugins::setup_frontend_plugins(&mut fe_opts).await.unwrap();
+        let mut plugins = Plugins::new();
+        plugins::setup_frontend_plugins(&mut plugins, &fe_opts)
+            .await
+            .unwrap();
 
         let provider = plugins.get::<UserProviderRef>().unwrap();
         let result = provider

diff --git a/src/cmd/src/metasrv.rs b/src/cmd/src/metasrv.rs
@@ -16,6 +16,7 @@ use std::time::Duration;
 
 use async_trait::async_trait;
 use clap::Parser;
+use common_base::Plugins;
 use common_config::Configurable;
 use common_telemetry::info;
 use common_telemetry::logging::TracingOptions;
@@ -238,8 +239,9 @@ impl StartCommand {
         info!("Metasrv start command: {:#?}", self);
         info!("Metasrv options: {:#?}", opts);
 
-        let mut opts = opts.component;
-        let plugins = plugins::setup_metasrv_plugins(&mut opts)
+        let opts = opts.component;
+        let mut plugins = Plugins::new();
+        plugins::setup_metasrv_plugins(&mut plugins, &opts)
             .await
             .context(StartMetaServerSnafu)?;
 

diff --git a/src/cmd/src/standalone.rs b/src/cmd/src/standalone.rs
@@ -19,6 +19,7 @@ use async_trait::async_trait;
 use cache::{build_fundamental_cache_registry, with_default_composite_cache_registry};
 use catalog::kvbackend::KvBackendCatalogManager;
 use clap::Parser;
+use common_base::Plugins;
 use common_catalog::consts::{MIN_USER_FLOW_ID, MIN_USER_TABLE_ID};
 use common_config::{metadata_store_dir, Configurable, KvBackendConfig};
 use common_error::ext::BoxedError;
@@ -418,14 +419,18 @@ impl StartCommand {
         info!("Standalone start command: {:#?}", self);
         info!("Standalone options: {opts:#?}");
 
+        let mut plugins = Plugins::new();
         let opts = opts.component;
-        let mut fe_opts = opts.frontend_options();
-        #[allow(clippy::unnecessary_mut_passed)]
-        let fe_plugins = plugins::setup_frontend_plugins(&mut fe_opts) // mut ref is MUST, DO NOT change it
+        let fe_opts = opts.frontend_options();
+        let dn_opts = opts.datanode_options();
+
+        plugins::setup_frontend_plugins(&mut plugins, &fe_opts)
             .await
             .context(StartFrontendSnafu)?;
 
-        let dn_opts = opts.datanode_options();
+        plugins::setup_datanode_plugins(&mut plugins, &dn_opts)
+            .await
+            .context(StartDatanodeSnafu)?;
 
         set_default_timezone(fe_opts.default_timezone.as_deref()).context(InitTimezoneSnafu)?;
 
@@ -464,15 +469,15 @@ impl StartCommand {
         let table_metadata_manager =
             Self::create_table_metadata_manager(kv_backend.clone()).await?;
 
-        let datanode = DatanodeBuilder::new(dn_opts, fe_plugins.clone())
+        let datanode = DatanodeBuilder::new(dn_opts, plugins.clone())
             .with_kv_backend(kv_backend.clone())
             .build()
             .await
             .context(StartDatanodeSnafu)?;
 
         let flow_builder = FlownodeBuilder::new(
             Default::default(),
-            fe_plugins.clone(),
+            plugins.clone(),
             table_metadata_manager.clone(),
             catalog_manager.clone(),
         );
@@ -533,7 +538,7 @@ impl StartCommand {
             node_manager.clone(),
             ddl_task_executor.clone(),
         )
-        .with_plugin(fe_plugins.clone())
+        .with_plugin(plugins.clone())
         .try_build()
         .await
         .context(StartFrontendSnafu)?;
@@ -554,7 +559,7 @@ impl StartCommand {
 
         let (tx, _rx) = broadcast::channel(1);
 
-        let servers = Services::new(fe_opts, Arc::new(frontend.clone()), fe_plugins)
+        let servers = Services::new(fe_opts, Arc::new(frontend.clone()), plugins)
             .build()
             .await
             .context(StartFrontendSnafu)?;
@@ -636,13 +641,15 @@ mod tests {
 
     #[tokio::test]
     async fn test_try_from_start_command_to_anymap() {
-        let mut fe_opts = FrontendOptions {
+        let fe_opts = FrontendOptions {
             user_provider: Some("static_user_provider:cmd:test=test".to_string()),
             ..Default::default()
         };
 
-        #[allow(clippy::unnecessary_mut_passed)]
-        let plugins = plugins::setup_frontend_plugins(&mut fe_opts).await.unwrap();
+        let mut plugins = Plugins::new();
+        plugins::setup_frontend_plugins(&mut plugins, &fe_opts)
+            .await
+            .unwrap();
 
         let provider = plugins.get::<UserProviderRef>().unwrap();
         let result = provider

diff --git a/src/plugins/src/datanode.rs b/src/plugins/src/datanode.rs
@@ -16,8 +16,13 @@ use common_base::Plugins;
 use datanode::config::DatanodeOptions;
 use datanode::error::Result;
 
-pub async fn setup_datanode_plugins(_opts: &mut DatanodeOptions) -> Result<Plugins> {
-    Ok(Plugins::new())
+#[allow(unused_variables)]
+#[allow(unused_mut)]
+pub async fn setup_datanode_plugins(
+    plugins: &mut Plugins,
+    dn_opts: &DatanodeOptions,
+) -> Result<()> {
+    Ok(())
 }
 
 pub async fn start_datanode_plugins(_plugins: Plugins) -> Result<()> {

diff --git a/src/plugins/src/frontend.rs b/src/plugins/src/frontend.rs
@@ -18,16 +18,17 @@ use frontend::error::{IllegalAuthConfigSnafu, Result};
 use frontend::frontend::FrontendOptions;
 use snafu::ResultExt;
 
-pub async fn setup_frontend_plugins(opts: &FrontendOptions) -> Result<Plugins> {
-    let plugins = Plugins::new();
-
-    if let Some(user_provider) = opts.user_provider.as_ref() {
+#[allow(unused_mut)]
+pub async fn setup_frontend_plugins(
+    plugins: &mut Plugins,
+    fe_opts: &FrontendOptions,
+) -> Result<()> {
+    if let Some(user_provider) = fe_opts.user_provider.as_ref() {
         let provider =
             auth::user_provider_from_option(user_provider).context(IllegalAuthConfigSnafu)?;
         plugins.insert::<UserProviderRef>(provider);
     }
-
-    Ok(plugins)
+    Ok(())
 }
 
 pub async fn start_frontend_plugins(_plugins: Plugins) -> Result<()> {

diff --git a/src/plugins/src/meta_srv.rs b/src/plugins/src/meta_srv.rs
@@ -16,8 +16,12 @@ use common_base::Plugins;
 use meta_srv::error::Result;
 use meta_srv::metasrv::MetasrvOptions;
 
-pub async fn setup_metasrv_plugins(_opts: &mut MetasrvOptions) -> Result<Plugins> {
-    Ok(Plugins::new())
+#[allow(unused_variables)]
+pub async fn setup_metasrv_plugins(
+    _plugins: &mut Plugins,
+    metasrv_opts: &MetasrvOptions,
+) -> Result<()> {
+    Ok(())
 }
 
 pub async fn start_metasrv_plugins(_plugins: Plugins) -> Result<()> {