mononoke/cmdlib: remove public-facing add_scribe_logging_args API

Summary:
Like it says in the title. There's no reason for this to be ad ad-hoc "throw in
an arg" when everything else is done by adding arg types.

Reviewed By: HarveyHunt

Differential Revision: D27791333

fbshipit-source-id: 38e5a479800179b249ace5cc599340cb84eb53e2

eden/mononoke/cmdlib/src/args/app.rs

@@ -50,6 +50,7 @@ pub const MYSQL_SQLBLOB_POOL_IDLE_TIMEOUT: &str = "mysql-sqblob-pool-idle-timeou
 pub const RUNTIME_THREADS: &str = "runtime-threads";
 pub const TUNABLES_CONFIG: &str = "tunables-config";
 pub const DISABLE_TUNABLES: &str = "disable-tunables";
+pub const SCRIBE_LOGGING_DIRECTORY: &str = "scribe-logging-directory";
 
 pub const READ_QPS_ARG: &str = "blobstore-read-qps";
 pub const WRITE_QPS_ARG: &str = "blobstore-write-qps";
@@ -116,6 +117,8 @@ pub enum ArgType {
     /// Adds --enable-mcrouter to use McRouter to talk to Memcache for places that support it,
     /// which can boot faster in dev binaries.
     McRouter,
+    /// Adds arguments related to Scribe logging.
+    Scribe,
 }
 
 // Arguments that are enabled by default for MononokeAppBuilder
@@ -337,6 +340,11 @@ impl MononokeAppBuilder {
         self
     }
 
+    pub fn with_scribe_args(mut self) -> Self {
+        self.arg_types.insert(ArgType::Scribe);
+        self
+    }
+
     pub fn with_default_scuba_dataset(mut self, default: impl Into<String>) -> Self {
         self.default_scuba_dataset = Some(default.into());
         self
@@ -546,6 +554,9 @@ impl MononokeAppBuilder {
         if self.arg_types.contains(&ArgType::McRouter) {
             app = add_mcrouter_args(app);
         }
+        if self.arg_types.contains(&ArgType::Scribe) {
+            app = add_scribe_logging_args(app);
+        }
 
         MononokeClapApp {
             clap: app,
@@ -989,10 +1000,10 @@ fn add_scuba_logging_args<'a, 'b>(app: App<'a, 'b>, has_default: bool) -> App<'a
     app
 }
 
-pub fn add_scribe_logging_args<'a, 'b>(app: MononokeClapApp<'a, 'b>) -> MononokeClapApp<'a, 'b> {
+fn add_scribe_logging_args<'a, 'b>(app: App<'a, 'b>) -> App<'a, 'b> {
     app.arg(
-        Arg::with_name("scribe-logging-directory")
-            .long("scribe-logging-directory")
+        Arg::with_name(SCRIBE_LOGGING_DIRECTORY)
+            .long(SCRIBE_LOGGING_DIRECTORY)
             .takes_value(true)
             .help("Filesystem directory where to log all scribe writes"),
     )

eden/mononoke/cmdlib/src/args/mod.rs

@@ -38,14 +38,11 @@ use sql_construct::SqlConstructFromMetadataDatabaseConfig;
 use crate::helpers::{setup_repo_dir, CreateStorage};
 
 use self::app::{
-    CONFIG_PATH, REPO_ID, REPO_NAME, SOURCE_REPO_ID, SOURCE_REPO_NAME, TARGET_REPO_ID,
-    TARGET_REPO_NAME,
+    CONFIG_PATH, REPO_ID, REPO_NAME, SCRIBE_LOGGING_DIRECTORY, SOURCE_REPO_ID, SOURCE_REPO_NAME,
+    TARGET_REPO_ID, TARGET_REPO_NAME,
 };
 
-// TODO: Move add_FOO_args into MononokeAppBuilder here.
-pub use self::app::{
-    add_scribe_logging_args, ArgType, MononokeAppBuilder, MononokeClapApp, RepoRequirement,
-};
+pub use self::app::{ArgType, MononokeAppBuilder, MononokeClapApp, RepoRequirement};
 pub use self::matches::MononokeMatches;
 
 fn get_repo_id_and_name_from_values<'a>(
@@ -339,7 +336,7 @@ pub fn get_shutdown_timeout<'a>(matches: &MononokeMatches<'a>) -> Result<Duratio
 }
 
 pub fn get_scribe<'a>(fb: FacebookInit, matches: &MononokeMatches<'a>) -> Result<Scribe> {
-    match matches.value_of("scribe-logging-directory") {
+    match matches.value_of(SCRIBE_LOGGING_DIRECTORY) {
         Some(dir) => Ok(Scribe::new_to_file(PathBuf::from(dir))),
         None => Ok(Scribe::new(fb)),
     }

eden/mononoke/scs_server/src/main.rs

@@ -65,6 +65,7 @@ fn main(fb: FacebookInit) -> Result<(), Error> {
         .with_shutdown_timeout_args()
         .with_scuba_logging_args()
         .with_disabled_hooks_args()
+        .with_scribe_args()
         .build()
         .arg(
             Arg::with_name(ARG_HOST)
@@ -83,7 +84,6 @@ fn main(fb: FacebookInit) -> Result<(), Error> {
                 .value_name("PORT")
                 .help("Thrift port"),
         );
-    let app = args::add_scribe_logging_args(app);
 
     let matches = app.get_matches(fb)?;
 

eden/mononoke/server/src/main.rs

@@ -35,12 +35,13 @@ const ARG_TICKET_SEEDS: &str = "ssl-ticket-seeds";
 const ARG_CSLB_CONFIG: &str = "cslb-config";
 
 fn setup_app<'a, 'b>() -> args::MononokeClapApp<'a, 'b> {
-    let app = args::MononokeAppBuilder::new("mononoke server")
+    args::MononokeAppBuilder::new("mononoke server")
         .with_shutdown_timeout_args()
         .with_all_repos()
         .with_disabled_hooks_args()
         .with_scuba_logging_args()
         .with_mcrouter_args()
+        .with_scribe_args()
         .with_default_scuba_dataset("mononoke_test_perf")
         .build()
         .about("serve repos")
@@ -91,10 +92,7 @@ fn setup_app<'a, 'b>() -> args::MononokeClapApp<'a, 'b> {
                 .takes_value(true)
                 .required(false)
                 .help("top level Mononoke tier where CSLB publishes routing table"),
-        );
-
-    let app = args::add_scribe_logging_args(app);
-    app
+        )
 }
 
 #[fbinit::main]