zhengruifeng commented on code in PR #53:
URL: https://github.com/apache/spark-connect-rust/pull/53#discussion_r3851439140


##########
.github/workflows/docs.yml:
##########
@@ -0,0 +1,61 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+name: Docs
+
+on:
+  push:
+    branches: [master, main]
+    paths:
+      - "docs/**"
+      - "mkdocs.yml"
+      - ".github/workflows/docs.yml"
+  workflow_dispatch:
+
+# `mkdocs gh-deploy` builds the site and pushes it to the `gh-pages` branch,
+# which ASF INFRA serves as GitHub Pages (see `.asf.yaml`: `ghp_branch:
+# gh-pages`). Only first-party `actions/*` are used, per the ASF Actions 
policy.
+permissions:
+  contents: write
+
+concurrency:
+  group: docs-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  build-and-deploy:
+    name: Build and deploy the docs site
+    runs-on: ubuntu-latest
+    # Only publish from the canonical repo, never from forks' scheduled runs.
+    if: github.event_name == 'workflow_dispatch' || github.ref == 
'refs/heads/master'

Review Comment:
   Require both the canonical repository and `refs/heads/master` for every 
deployment. The current `||` lets manual dispatches publish any selected 
branch, and fork-master runs also pass despite the comment; use a conjunction 
with `github.repository == 'apache/spark-connect-rust'` and the master-ref 
check.



##########
scripts/gen_parity_ledger.py:
##########
@@ -10,10 +10,10 @@
 Usage:
     python scripts/gen_parity_ledger.py \
         --src ~/workspace/origin/spark/python/pyspark \
-        --out docs/parity
+        --out dev/parity
 
-Status of each item is tracked in ``docs/parity/status.csv`` (hand/tool 
edited);
-this script only (re)generates the *inventory* (``docs/parity/inventory.csv``)
+Status of each item is tracked in ``dev/parity/status.csv`` (hand/tool edited);

Review Comment:
   Document `dev/parity/inventory.csv` as the status source. `main()` reads 
prior `status` and `notes` from `inventory.csv` and rewrites that file; it 
never opens `status.csv`, so this currently sends maintainers to a file the 
generator ignores.



##########
docs/types.md:
##########
@@ -0,0 +1,127 @@
+# Types and Schemas
+
+Spark SQL uses a rich type system. Learn how to work with schemas in the 
`spark_connect` crate and cast columns between types.
+
+## Spark SQL Types
+
+Map Spark SQL types to their Rust `spark_connect::types::DataType` equivalents:
+
+| Spark Type | Rust DataType |
+|-----------|---------|
+| StringType | `DataType::String { collation: "UTF8_BINARY".to_string() }` |

Review Comment:
   Update this table and the schema snippets to the actual enum variants. 
`String` requires `collation`; `Array` and `Map` are struct variants with 
nullability fields; structs use `DataType::Struct`; and `StructField` requires 
`metadata`. The current examples cannot compile against this crate.



##########
docs/quickstart.md:
##########
@@ -0,0 +1,72 @@
+# Quickstart
+
+Get up and running with Spark Connect in minutes. This guide walks you through 
your first Rust query.
+
+## Prerequisites
+
+Before you start, ensure you have a running Spark Connect server. See 
[Configuration and Connection](configuration.md) for how to start one locally:
+
+```bash
+$SPARK_HOME/sbin/start-connect-server.sh --packages 
"org.apache.spark:spark-connect_2.13:4.2.0"
+```
+
+The server listens on `sc://localhost:15002` by default.
+
+## Your First Query
+
+Connect to the server and run a simple query:
+
+```rust
+use spark_connect::SparkSession;
+
+fn main() -> Result<(), Box<dyn std::error::Error>> {

Review Comment:
   Rewrite this and the other Rust guide snippets for the crate's synchronous 
API. `get_or_create`, `range`, and `show`/`count` return `Result` directly; 
`range` accepts one end argument and `show` accepts `usize`, so the 
`#[tokio::main]`, `.await`, two-argument `range`, and `show(None)` forms here 
do not compile. The same async pattern appears in `configuration.md`, 
`dataframes.md`, `examples.md`, and `sql.md`.



##########
docs/sql.md:
##########
@@ -0,0 +1,152 @@
+# SQL
+
+Execute SQL directly against DataFrames and data sources. Mix SQL queries with 
the DataFrame API for maximum flexibility.
+
+## Running SQL Queries
+
+Use `spark.sql()` to execute SQL and retrieve results as a DataFrame.
+
+```rust
+use spark_connect::SparkSession;
+
+let spark = SparkSession::builder()
+    .remote("sc://localhost:15002")
+    .get_or_create()?;
+
+// Simple query
+let df = spark.sql("SELECT 1 as id, 'hello' as msg")?;
+df.show(10)?;
+
+// Aggregate query
+let df = spark.sql(
+    r#"SELECT category, COUNT(*) as cnt, AVG(price) as avg_price
+       FROM products
+       GROUP BY category
+       ORDER BY cnt DESC"#
+)?;
+df.show(10)?;
+```
+
+## Registering Temporary Views
+
+Make DataFrames queryable via SQL by creating temporary views.
+
+```rust
+use spark_connect::SparkSession;
+
+let spark = SparkSession::builder()
+    .remote("sc://localhost:15002")
+    .get_or_create()?;
+
+// Create from range
+let df = spark.range(4)?;
+
+// Register as temp view
+df.create_or_replace_temp_view("users")?;
+
+// Query it
+let result = spark.sql("SELECT * FROM users WHERE id > 1")?;
+result.show(10)?;
+
+// Replace view
+let df_updated = spark.sql("SELECT id FROM users")?;
+df_updated.create_or_replace_temp_view("users")?;
+```
+
+!!! note
+    Temporary views are scoped to the session and are dropped when the session 
ends.
+
+## Parameterized SQL
+
+Pass dynamic values into SQL queries safely using parameter binding.
+
+```rust
+use spark_connect::SparkSession;
+

Review Comment:
   Do not describe `format!` interpolation as safe parameter binding. The 
example inserts `category` inside quoted SQL, so caller-controlled input can 
break syntax or inject SQL, while `SparkSession::sql` currently accepts only a 
query string. Use typed DataFrame predicates or document a real parameter API.



##########
docs/configuration.md:
##########
@@ -0,0 +1,78 @@
+# Configuration and Connection
+
+Configure connections to local or remote Spark Connect servers, set session 
parameters, and manage authentication.
+
+## Connection String Format
+
+Connection strings follow the format `sc://host:port/;param=value`. Parameters 
are optional:
+
+| Parameter | Purpose | Example |
+| --- | --- | --- |
+| `token` | Authentication token | `sc://host:15002/;token=abc123` |
+| `user_id` | User identity | `sc://host:15002/;user_id=alice` |
+| `session_id` | Session identifier | `sc://host:15002/;session_id=sess_xyz` |
+| `use_ssl` | Enable TLS | `sc://host:15002/;use_ssl=true` |
+
+!!! note
+    TLS and authentication parameters are passed in the connection string. 
Consult your infrastructure team for any required credentials or certificates.
+
+## Building a Session
+
+```rust
+use spark_connect::SparkSessionBuilder;
+
+let spark = SparkSessionBuilder::default()
+    .remote("sc://localhost:15002")
+    .get_or_create()?;
+
+// Session configuration is applied at runtime via `spark.conf()`:
+spark.conf().set("spark.sql.shuffle.partitions", "20")?;
+```
+
+## Runtime Configuration
+
+Access and modify session configuration at runtime:
+
+```rust
+// Get a config value
+let partitions = spark.conf().get("spark.sql.shuffle.partitions")?;
+
+// Set a config value
+spark.conf().set("spark.sql.adaptive.enabled", "true")?;
+```
+
+## Starting a Local Spark Connect Server
+
+To run a Spark Connect server locally for development:
+
+```bash
+# Set SPARK_HOME to your Spark installation
+export SPARK_HOME=/path/to/spark
+
+# Start the server on sc://localhost:15002
+$SPARK_HOME/sbin/start-connect-server.sh \
+  --packages "org.apache.spark:spark-connect_2.13:4.2.0"
+```
+
+The server listens on port `15002` by default. To use a different port, add 
`--conf spark.connect.grpc.binding.port=<port>`.
+
+!!! tip
+    The Spark Connect server requires a JVM and Apache Spark 4.2.0 or later. 
Stop it with `$SPARK_HOME/sbin/stop-connect-server.sh`.
+
+## Remote Connections
+
+To connect to a remote Spark Connect server:
+
+```rust
+let spark = SparkSessionBuilder::default()

Review Comment:
   Keep the `/;` delimiter before connection parameters. `ChannelBuilder` 
begins parameter parsing only after `/`; without it, `;token=...` stays in the 
authority and port parsing fails. Fix this URL, the TLS example below, and the 
matching troubleshooting example to use `sc://host:15002/;...`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to