Fix #244 - Remove CSV and old-format JSON formatting

Author: Lars T Hansen

README.md

@@ -59,11 +59,8 @@ are known to work for both.  Also see comments in `gpuapi/Makefile`.
 
 ## Output format options
 
-There are two output formats, [the old format](doc/OLD-FORMAT.md) and [the new
-format](doc/NEW-FORMAT.md), currently coexisting but the old format will be phased out.
-
-The recommended (and default as of v0.16) output format is the "new" JSON format.  There are command
-line switches to force the older formats, CSV or an older JSON format.
+As of v0.17 there is only one output format, known as [the new format](doc/NEW-FORMAT.md), a JSON
+encoding.  Support for the older CSV format and an older JSON encoding have been removed.
 
 ## Examples
 
@@ -75,34 +72,19 @@ cluster", below.  For a full description of the output formats and fields, see t
 It's sensible to run `sonar ps` every 5 minutes on every compute node if you care mostly about
 long-running jobs, or at higher frequency if sbrief jobs are of interest to you.
 
-Here is an example output (with the older CSV output format):
+Here is an example output:
 ```console
 $ sonar ps --exclude-system-jobs --min-cpu-time=10
-
-v=0.7.0,time=2023-08-10T11:09:41+02:00,host=somehost,cores=8,user=someone,job=0,cmd=fish,cpu%=2.1,cpukib=64400,gpus=none,gpu%=0,gpumem%=0,gpukib=0,cputime_sec=138
-v=0.7.0,time=2023-08-10T11:09:41+02:00,host=somehost,cores=8,user=someone,job=0,cmd=sonar,cpu%=761,cpukib=372,gpus=none,gpu%=0,gpumem%=0,gpukib=0,cputime_sec=137
-v=0.7.0,time=2023-08-10T11:09:41+02:00,host=somehost,cores=8,user=someone,job=0,cmd=brave,cpu%=14.6,cpukib=2907168,gpus=none,gpu%=0,gpumem%=0,gpukib=0,cputime_sec=3532
-v=0.7.0,time=2023-08-10T11:09:41+02:00,host=somehost,cores=8,user=someone,job=0,cmd=alacritty,cpu%=0.8,cpukib=126700,gpus=none,gpu%=0,gpumem%=0,gpukib=0,cputime_sec=51
-v=0.7.0,time=2023-08-10T11:09:41+02:00,host=somehost,cores=8,user=someone,job=0,cmd=pulseaudio,cpu%=0.7,cpukib=90640,gpus=none,gpu%=0,gpumem%=0,gpukib=0,cputime_sec=399
-v=0.7.0,time=2023-08-10T11:09:41+02:00,host=somehost,cores=8,user=someone,job=0,cmd=slack,cpu%=3.9,cpukib=716924,gpus=none,gpu%=0,gpumem%=0,gpukib=0,cputime_sec=266
+...
 ```
 
 ### Collect system information with `sonar sysinfo`
 
-The `sysinfo` subcommand collects information about the system and prints it in JSON form on stdout
-(this is the older JSON format):
+The `sysinfo` subcommand collects information about the system and prints it on stdout.
 
 ```console
 $ sonar sysinfo
-{
- "timestamp": "2024-02-26T00:00:02+01:00",
- "hostname": "ml1.hpc.uio.no",
- "description": "2x14 (hyperthreaded) Intel(R) Xeon(R) Gold 5120 CPU @ 2.20GHz, 125 GB, 3x NVIDIA GeForce RTX 2080 Ti @ 11GB",
- "cpu_cores": 56,
- "mem_gb": 125,
- "gpu_cards": 3,
- "gpumem_gb": 33
-}
+...
 ```
 
 Typical usage for `sysinfo` is to run the command after reboot and (for hot-swappable systems and

doc/CHANGELOG.md

@@ -17,6 +17,9 @@ should ask for that format.
 
 ## Changes in v0.17.0 (on `main`)
 
+* Bug 244 - **REMOVED FUNCTIONALITY.** Remove all code related to producing CSV and the "old"
+  JSON format, asking for those formats will now cause errors.  The --json switch is still
+  accepted but has no effect.
 * Bug 434 - **IMPORTANT FUNCTIONALITY.** Introduce the batch-size setting to allow very large
   slurm job lists to be split into multiple messages.
 

doc/OLD-FORMAT.md

@@ -294,7 +294,7 @@ pub fn daemon_mode(
             exclude_commands: ini.sample.exclude_commands.clone(),
             lockdir: ini.global.lockdir.clone(),
             token: api_token.clone(),
-            fmt: ps::Format::NewJSON,
+            fmt: ps::Format::JSON,
             cpu_util: true,
             min_cpu_percent: None, // Nonmonotonic, considered obsolete
             min_mem_percent: None, // Nonmonotonic, considered obsolete

src/daemon.rs

@@ -18,7 +18,6 @@ mod output;
 mod output_test;
 mod ps;
 mod ps_newfmt;
-mod ps_oldfmt;
 #[cfg(test)]
 mod ps_test;
 mod slurmjobs;
@@ -77,9 +76,6 @@ enum Commands {
         /// the per-cpu usage since boot.
         load: bool,
 
-        /// Output old CSV, not new JSON
-        csv: bool,
-
         /// Cluster name
         cluster: Option<String>,
     },
@@ -90,12 +86,6 @@ enum Commands {
     },
     /// Extract node information
     Sysinfo {
-        /// Output CSV, not old JSON
-        csv: bool,
-
-        /// Output old JSON, not new JSON
-        oldjson: bool,
-
         /// Cluster name
         cluster: Option<String>,
 
@@ -116,9 +106,6 @@ enum Commands {
         /// to is exclusive.  Precludes -window.
         span: Option<String>,
 
-        /// Output old CSV, not new JSON
-        csv: bool,
-
         /// Include PENDING and RUNNING jobs
         deluge: bool,
 
@@ -169,7 +156,6 @@ fn main() {
             exclude_commands,
             lockdir,
             load,
-            csv,
             cluster,
         } => {
             let opts = ps::PsOptions {
@@ -190,11 +176,7 @@ fn main() {
                     vec![]
                 },
                 lockdir: lockdir.clone(),
-                fmt: if *csv {
-                    ps::Format::CSV
-                } else {
-                    ps::Format::NewJSON
-                },
+                fmt: ps::Format::JSON,
                 cpu_util: true,
                 token,
             };
@@ -212,8 +194,6 @@ fn main() {
             );
         }
         Commands::Sysinfo {
-            csv,
-            oldjson,
             cluster,
             topo_svg_cmd,
             topo_text_cmd,
@@ -227,21 +207,14 @@ fn main() {
                 writer,
                 &system.freeze().expect("System initialization"),
                 token,
-                if *csv {
-                    sysinfo::Format::CSV
-                } else if *oldjson {
-                    sysinfo::Format::OldJSON
-                } else {
-                    sysinfo::Format::NewJSON
-                },
+                sysinfo::Format::JSON,
                 topo_svg_cmd.clone(),
                 topo_text_cmd.clone(),
             );
         }
         Commands::Slurmjobs {
             window,
             span,
-            csv,
             deluge,
             batch_size,
             cluster,
@@ -259,11 +232,7 @@ fn main() {
                 *batch_size,
                 &system.freeze().expect("System initialization"),
                 token,
-                if *csv {
-                    slurmjobs::Format::CSV
-                } else {
-                    slurmjobs::Format::NewJSON
-                },
+                slurmjobs::Format::JSON,
             );
         }
         Commands::Cluster { cluster } => {
@@ -288,10 +257,7 @@ fn main() {
 // For the sake of simplicity:
 //  - allow repeated options to overwrite earlier values
 //  - all error reporting is via a generic "usage" message, without specificity as to what was wrong
-//  - both --json and --csv are accepted to all commands
-//
-// Note that --json means "new json" everywhere, so --json for `sonar sysinfo` changes the output
-// format from the default old JSON encoding.
+//  - --json does nothing, while --csv and --oldfmt cause errors
 
 fn command_line() -> Commands {
     let args = std::env::args().collect::<Vec<String>>();
@@ -322,8 +288,6 @@ fn command_line() -> Commands {
                 let mut exclude_commands = None;
                 let mut lockdir = None;
                 let mut load = false;
-                let mut json = false;
-                let mut csv = false;
                 let mut cluster = None;
                 while next < args.len() {
                     let arg = args[next].as_ref();
@@ -336,11 +300,7 @@ fn command_line() -> Commands {
                     } else if let Some(new_next) = bool_arg(arg, &args, next, "--load") {
                         (next, load) = (new_next, true);
                     } else if let Some(new_next) = bool_arg(arg, &args, next, "--json") {
-                        (next, json) = (new_next, true);
-                    } else if let Some(new_next) = bool_arg(arg, &args, next, "--csv") {
-                        (next, csv) = (new_next, true);
-                    } else if let Some(new_next) = bool_arg(arg, &args, next, "--oldfmt") {
-                        (next, csv) = (new_next, true);
+                        next = new_next;
                     } else if let Some(new_next) =
                         bool_arg(arg, &args, next, "--exclude-system-jobs")
                     {
@@ -377,10 +337,6 @@ fn command_line() -> Commands {
                         usage(true);
                     }
                 }
-                if json && csv {
-                    eprintln!("--csv and --json are incompatible");
-                    std::process::exit(USAGE_ERROR);
-                }
                 Commands::PS {
                     rollup,
                     min_cpu_percent,
@@ -391,26 +347,18 @@ fn command_line() -> Commands {
                     exclude_commands,
                     lockdir,
                     load,
-                    csv,
                     cluster,
                 }
             }
             "sysinfo" => {
-                let mut json = false;
-                let mut oldjson = false;
-                let mut csv = false;
                 let mut cluster = None;
                 let mut topo_svg_cmd = None;
                 let mut topo_text_cmd = None;
                 while next < args.len() {
                     let arg = args[next].as_ref();
                     next += 1;
                     if let Some(new_next) = bool_arg(arg, &args, next, "--json") {
-                        (next, json) = (new_next, true);
-                    } else if let Some(new_next) = bool_arg(arg, &args, next, "--oldfmt") {
-                        (next, oldjson) = (new_next, true);
-                    } else if let Some(new_next) = bool_arg(arg, &args, next, "--csv") {
-                        (next, csv) = (new_next, true);
+                        next = new_next;
                     } else if let Some((new_next, value)) =
                         string_arg(arg, &args, next, "--cluster")
                     {
@@ -427,17 +375,7 @@ fn command_line() -> Commands {
                         usage(true);
                     }
                 }
-                if (json || oldjson) && csv {
-                    eprintln!("--csv is incompatible with --json and --oldfmt");
-                    std::process::exit(USAGE_ERROR);
-                }
-                if json && oldjson {
-                    eprintln!("--json and --oldfmt are incompatible");
-                    std::process::exit(USAGE_ERROR);
-                }
                 Commands::Sysinfo {
-                    csv,
-                    oldjson,
                     cluster,
                     topo_svg_cmd,
                     topo_text_cmd,
@@ -446,8 +384,6 @@ fn command_line() -> Commands {
             "slurm" => {
                 let mut window = None;
                 let mut span = None;
-                let mut json = false;
-                let mut csv = false;
                 let mut deluge = false;
                 let mut batch_size = None;
                 let mut cluster = None;
@@ -461,11 +397,7 @@ fn command_line() -> Commands {
                     } else if let Some((new_next, value)) = string_arg(arg, &args, next, "--span") {
                         (next, span) = (new_next, Some(value));
                     } else if let Some(new_next) = bool_arg(arg, &args, next, "--json") {
-                        (next, json) = (new_next, true);
-                    } else if let Some(new_next) = bool_arg(arg, &args, next, "--oldfmt") {
-                        (next, csv) = (new_next, true);
-                    } else if let Some(new_next) = bool_arg(arg, &args, next, "--csv") {
-                        (next, csv) = (new_next, true);
+                        next = new_next;
                     } else if let Some(new_next) = bool_arg(arg, &args, next, "--deluge") {
                         (next, deluge) = (new_next, true);
                     } else if let Some((new_next, value)) =
@@ -483,14 +415,9 @@ fn command_line() -> Commands {
                 if window.is_some() && span.is_some() {
                     usage(true);
                 }
-                if json && csv {
-                    eprintln!("--csv and --json are incompatible");
-                    std::process::exit(USAGE_ERROR);
-                }
                 Commands::Slurmjobs {
                     window,
                     span,
-                    csv,
                     cluster,
                     deluge,
                     batch_size,
@@ -504,9 +431,6 @@ fn command_line() -> Commands {
                     if let Some(new_next) = bool_arg(arg, &args, next, "--json") {
                         // Ignore, there is only one format
                         next = new_next;
-                    } else if let Some(new_next) = bool_arg(arg, &args, next, "--oldfmt") {
-                        // Ignore, there is only one format
-                        next = new_next;
                     } else if let Some((new_next, value)) =
                         string_arg(arg, &args, next, "--cluster")
                     {
@@ -623,18 +547,10 @@ Options for `ps`:
       exists on startup [default: none]
   --load
       Print per-cpu and per-gpu load data
-  --csv
-      Format output as old CSV, not JSON
-  --oldfmt
-      Synonym for --csv
   --cluster name
       Optional cluster name with which to tag output
 
 Options for `sysinfo`:
-  --csv
-      Format output as CSV, not JSON
-  --oldfmt
-      Format output as the old JSON, not the new JSON
   --cluster name
       Optional cluster name with which to tag output
   --topo-svg-cmd
@@ -655,10 +571,6 @@ Options for `slurm`:
       Include PENDING and RUNNING jobs in the output, not just completed jobs.
   --batch-size
       Split into multiple JSON messages after this many job records.
-  --csv
-      Format output as CSV, not new JSON
-  --oldfmt
-      Synonym for --csv
   --cluster name
       Optional cluster name with which to tag output