feat: Add spice user agent, update for clippy rules

.github/workflows/build.yml

@@ -10,15 +10,15 @@ env:
   CARGO_TERM_COLOR: always
 
 jobs:
-  build_and_test:
+  build_toolchains:
     runs-on: ubuntu-latest
-    strategy:
-      matrix:
-        toolchain:
-          - stable
-          - beta
-          - nightly
-
+      strategy:
+        fail-fast: false
+        matrix:
+          toolchain:
+            - stable
+            - beta
+            - nightly
     steps:
       - uses: actions/checkout@v4
 
@@ -45,24 +45,84 @@ jobs:
           sarif_file: rust-clippy-results.sarif
           wait-for-processing: true
 
-      - name: Build
-        run: cargo build --verbose
+  build_and_test:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        os: [ubuntu-latest, macos-latest, windows-latest]
+        toolchain:
+          - stable
+          - beta
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install Rust toolchain
+        uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: ${{ matrix.toolchain }}
+          components: clippy
+          override: true
+
+      - name: Cargo clippy
+        run: cargo clippy --all-features
 
-      - name: install Spice (https://install.spiceai.org)
+      - name: Install Spice (https://install.spiceai.org) (Linux)
+        if: matrix.os == 'ubuntu-latest'
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         run: |
           curl https://install.spiceai.org | /bin/bash
           echo "$HOME/.spice/bin" >> $GITHUB_PATH
+          $HOME/.spice/bin/spice install
+      
+      - name: Install Spice (https://install.spiceai.org) (MacOS)
+        if: matrix.os == 'macos-latest'
+        run: |
+          brew install spiceai/spiceai/spice
+          brew install spiceai/spiceai/spiced
+
+      - name: install Spice (Windows)
+        if: matrix.os == 'windows-latest'
+        run: |
+          curl -L "https://install.spiceai.org/Install.ps1" -o Install.ps1 && PowerShell -ExecutionPolicy Bypass -File ./Install.ps1
+
+      - name: add Spice bin to PATH (Windows)
+        if: matrix.os == 'windows-latest'
+        run: |
+          Add-Content $env:GITHUB_PATH (Join-Path $HOME ".spice\bin")
+        shell: pwsh
 
       - name: Init and start spice app
+        if: matrix.os != 'windows-latest'
+        run: |
+          spice init spice_qs
+          cd spice_qs
+          spice add spiceai/quickstart
+          spiced &> spice.log &
+          # time to initialize added dataset
+          sleep 5
+
+      - name: Init and start spice app (Windows)
+        if: matrix.os == 'windows-latest'
         run: |
           spice init spice_qs
           cd spice_qs
           spice add spiceai/quickstart
-          spice run &> spice.log &
+          Start-Process -FilePath spice run
           # time to initialize added dataset
-          sleep 10
+          Start-Sleep -Seconds 5
+        shell: pwsh
 
       - name: Run tests
         run: cargo test --verbose
         env:
           API_KEY: ${{ secrets.TEST_API_KEY }}
+
+      - name: Stop spice and check logs
+        working-directory: spice_qs
+        if: matrix.os != 'windows-latest' && always()
+        run: |
+          killall spice || true
+          cat spice.log

Cargo.toml

@@ -27,3 +27,6 @@ dotenv = "0.15.0"
 arrow = "51.0.0"
 futures = "0.3.30"
 base64 = "0.22.0"
+
+[dev-dependencies]
+regex = "1.10.6"

README.md

@@ -33,7 +33,7 @@ async fn main() {
 }
 ```
 
-### New client with https://spice.ai cloud
+### New client with <https://spice.ai> cloud
 
 ```rust
 use spiceai::ClientBuilder;

src/client.rs

@@ -49,6 +49,10 @@ impl SpiceClient {
     ///     let mut client = Client::new("API_KEY").await.unwrap();
     /// }
     /// ```
+    ///
+    /// ## Errors
+    ///
+    /// - `Box<dyn Error>` for any query error
     pub async fn new(api_key: &str) -> Result<Self, Box<dyn Error>> {
         let config = SpiceClientConfig::load_from_default().await?;
 
@@ -58,6 +62,7 @@ impl SpiceClient {
         })
     }
 
+    #[must_use]
     pub fn builder() -> SpiceClientBuilder {
         SpiceClientBuilder::new()
     }
@@ -72,6 +77,10 @@ impl SpiceClient {
     /// let data = client.query("SELECT * FROM eth.recent_blocks LIMIT 10;").await;
     /// # }
     /// ````
+    ///
+    /// ## Errors
+    ///
+    /// - `Box<dyn Error>` for any query error
     pub async fn query(&mut self, query: &str) -> Result<FlightRecordBatchStream, Box<dyn Error>> {
         self.flight.query(query).await
     }
@@ -86,6 +95,10 @@ impl SpiceClient {
     /// let data = client.fire_query("SELECT * FROM eth.recent_blocks LIMIT 10;").await;
     /// # }
     /// ````
+    ///
+    /// ## Errors
+    ///
+    /// - `Box<dyn Error>` for any query error
     pub async fn fire_query(
         &mut self,
         query: &str,
@@ -138,6 +151,7 @@ impl Default for SpiceClientBuilder {
 }
 
 impl SpiceClientBuilder {
+    #[must_use]
     pub fn new() -> Self {
         Self {
             api_key: None,
@@ -147,32 +161,40 @@ impl SpiceClientBuilder {
     }
 
     /// Configures the `SpiceClient` to use the given API key.
+    #[must_use]
     pub fn api_key(mut self, api_key: &str) -> Self {
         self.api_key = Some(api_key.to_string());
         self
     }
 
     /// Configures the `SpiceClient` to use the given Spice Firecache endpoint.
+    #[must_use]
     pub fn firecache_url(mut self, firecache_url: &str) -> Self {
         self.firecache_url = Some(firecache_url.to_string());
         self
     }
 
     /// Configures the `SpiceClient` to use the given Spice Flight endpoint.
+    #[must_use]
     pub fn flight_url(mut self, flight_url: &str) -> Self {
         self.flight_url = Some(flight_url.to_string());
         self
     }
 
     /// Configures the `SpiceClient` to use default Spice.ai Cloud endpoints.
     /// Equivalent to calling `.firecache_url("https://firecache.spiceai.io")` and `.flight_url("https://flight.spiceai.io")`.
+    #[must_use]
     pub fn use_spiceai_cloud(mut self) -> Self {
         self.flight_url = Some(SPICE_CLOUD_FLIGHT_ADDR.to_string());
         self.firecache_url = Some(SPICE_CLOUD_FIRECACHE_ADDR.to_string());
         self
     }
 
     /// Builds the `SpiceClient` with the specified configuration.
+    ///
+    /// ## Errors
+    ///
+    /// - `Box<dyn Error>` if flight or firecache channel creation fails
     pub async fn build(self) -> Result<SpiceClient, Box<dyn Error>> {
         let flight_channel = match self.flight_url {
             Some(url) => new_tls_flight_channel(&url).await?,

src/config.rs

@@ -3,3 +3,67 @@ pub const SPICE_CLOUD_FIRECACHE_ADDR: &str = "https://firecache.spiceai.io";
 
 // default address for local spice runtime
 pub const SPICE_LOCAL_FLIGHT_ADDR: &str = "http://localhost:50051";
+
+#[cfg(target_family = "unix")]
+fn get_os_release() -> Result<String, Box<dyn std::error::Error>> {
+    // call uname -r to get release text
+    use std::process::Command;
+    let output = Command::new("uname").arg("-r").output()?;
+    let release = String::from_utf8(output.stdout)?;
+
+    Ok(release)
+}
+
+#[cfg(target_family = "windows")]
+fn get_os_release() -> Result<String, Box<dyn std::error::Error>> {
+    todo!("get_os_release not implemented for Windows")
+}
+
+pub(crate) fn get_user_agent() -> String {
+    let os_type = std::env::consts::OS;
+    let os_type = if os_type.is_empty() {
+        "unknown".to_string()
+    } else {
+        // capitalize first letter
+        let mut os_type = os_type.to_string();
+        os_type[..1].make_ascii_uppercase();
+        os_type
+    };
+
+    let os_arch = std::env::consts::ARCH;
+    let os_arch = match os_arch {
+        "" => "unknown".to_string(),
+        "x86" => "i386".to_string(),
+        _ => os_arch.to_string(),
+    };
+
+    let os_release = get_os_release()
+        .unwrap_or_else(|_| "unknown".to_string())
+        .trim()
+        .to_string();
+
+    format!(
+        "spice-rs {} ({os_type}/{os_release} {os_arch})",
+        env!("CARGO_PKG_VERSION")
+    )
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn test_get_user_agent() {
+        let matching_regex = regex::Regex::new(
+            r"spice-rs \d+\.\d+\.\d+ \((Linux|Windows|macOS)/[\d\w\.\-\_]+ (x86_64|aarch64|i386)\)",
+        )
+        .expect("regex should be constructed");
+
+        let user_agent = get_user_agent();
+        let agent_matches = matching_regex.is_match(&user_agent);
+        assert!(
+            agent_matches,
+            "expected user agent to match regex, but got {user_agent}"
+        );
+    }
+}

src/flight.rs

@@ -1,3 +1,4 @@
+use crate::config::get_user_agent;
 use arrow::error::ArrowError;
 use arrow_flight::decode::FlightRecordBatchStream;
 use arrow_flight::error::FlightError;
@@ -24,24 +25,28 @@ pub struct SqlFlightClient {
     api_key: Option<String>,
 }
 
+#[allow(clippy::needless_pass_by_value)]
 fn status_to_arrow_error(status: tonic::Status) -> ArrowError {
     ArrowError::IpcError(format!("{status:?}"))
 }
 
 impl SqlFlightClient {
     pub fn new(chan: Channel, api_key: Option<String>) -> Self {
+        let mut headers = HashMap::new();
+        headers.insert("x-spice-user-agent".to_string(), get_user_agent());
+
         SqlFlightClient {
             api_key,
+            headers,
             client: FlightServiceClient::new(chan),
-            headers: HashMap::default(),
             token: None,
         }
     }
 
     async fn handshake(&mut self, username: &str, password: &str) -> Result<Bytes, ArrowError> {
         let cmd = HandshakeRequest {
             protocol_version: 0,
-            payload: Default::default(),
+            payload: Bytes::default(),
         };
         let mut req = tonic::Request::new(stream::iter(vec![cmd]));
         let val = BASE64_STANDARD.encode(format!("{username}:{password}"));

tests/client_test.rs

@@ -41,13 +41,13 @@ mod tests {
                             assert_eq!(batch.num_rows(), 10);
                         },
                         Err(e) => {
-                            panic!("Error: {}", e)
+                            panic!("Error: {e}")
                         },
                     };
                     }
                 }
                 Err(e) => {
-                    panic!("Error: {}", e);
+                    panic!("Error: {e}");
                 }
             };
     }
@@ -67,13 +67,13 @@ mod tests {
                             assert_eq!(batch.num_rows(), 10);
                         },
                         Err(e) => {
-                            panic!("Error: {}", e)
+                            panic!("Error: {e}")
                         }
                     };
                 }
             }
             Err(e) => {
-                panic!("Error: {}", e);
+                panic!("Error: {e}");
             }
         };
     }
@@ -95,15 +95,15 @@ mod tests {
                             total_rows += batch.num_rows();
                         },
                         Err(e) => {
-                            panic!("Error: {}", e)
+                            panic!("Error: {e}")
                         },
                     };
                     }
                     assert_eq!(total_rows, 2000);
                     assert_ne!(num_batches, 1);
                 }
                 Err(e) => {
-                    panic!("Error: {}", e);
+                    panic!("Error: {e}");
                 }
             };
     }