chore(py-glaredb): keyword args for connect

[greyscaled]

This alters the pyFunction connect to have two additional optional arguments disable_tls and cloud_addr. These arguments match those that are defined in LocalClientOpts.

Before

glaredb.connect([data_dir_or_cloud_url], [spill_path])

After

data_dir_or_cloud_url is positional, and all other args are keyword with defaults and variable in order

glaredb.connect([data_dir_or_cloud_url],
	[spill_path],
	[disable_tls],
	[cloud_addr]
)

Example Uses

Local

import glaredb

conn = glaredb.connect()
conn.sql("SELECT 'hello local';").show()
┌─────────────────────┐
│ Utf8("hello local") │
│ ──                  │
│ Utf8                │
╞═════════════════════╡
│ hello local         │
└─────────────────────┘

Hybrid (using keyword args in any order)

import glaredb

cloud_url = "<user>:<pass>@<org>.remote.qa.glaredb.com:6443/<db_name>"

conn = glaredb.connect(cloud_url, cloud_addr="https://qa.glaredb.com", disable_tls=False)
conn.sql("SELECT 'hello remote';").show()
┌──────────────────────┐
│ Utf8("hello remote") │
│ ──                   │
│ Utf8                 │
╞══════════════════════╡
│ hello remote         │
└──────────────────────┘

Resolves: #1829

[scsmithr]

Ideally these (and spill path) would be kwargs. Otherwise this will get very unwieldy.

https://pyo3.rs/main/function/signature#using-pyo3signature--

[greyscaled]

I'll make that improvement, thanks

[greyscaled]

IMO I like what I got it. Do you see any benefit of **kwargs over this?

#[pyo3(signature = (data_dir_or_cloud_url = None, /, *, spill_path = None, disable_tls = true, cloud_addr = String::from("https://console.glaredb.com")))]
fn connect(
    py: Python,
    data_dir_or_cloud_url: Option<String>,
    spill_path: Option<String>,
    disable_tls: bool,
    cloud_addr: String,
) -> PyResult<LocalSession> { }

• / defines the end of positional args
• * defines the beginning of keyword-only args (it's a separate comma because you can't do *arg=val)

So this means:

• data_dir_or_cloud_url is always the first (positional) arg
• order after that doesn't matter
• all these extra opts have default values and are hard to misconfigure, because by the nature of using a keyword, you have to provide a value!

LMK if you disagree on this. My thinking is that if we did

#[pyo3(signature = (data_dir_or_cloud_url = None, /, **kwargs))]
fn connect(
    py: Python,
    data_dir_or_cloud_url: Option<String>,
    opts: Option<&PyDict>,
) -> PyResult<LocalSession> { }

There would be additional plumbing on opts that I rather not muck with

[greyscaled]

Example use:

import glaredb

cloud_url = "<user>:<pass>@<org>.remote.qa.glaredb.com:6443/<db_name>"

conn = glaredb.connect(cloud_url, cloud_addr="https://qa.glaredb.com", disable_tls=False)

[scsmithr]

This looks reasonable to me, pinging @universalmind303 to see if he has opinions here.

[universalmind303]

This seems reasonable to me.

A question not really related to this PR, but why do we need cloud_url and cloud_addr? couldn't we infer the addr from the url?

[greyscaled]

@universalmind303

why do we need cloud_url and cloud_addr? couldn't we infer the addr from the url?

This is a good question and something I originally inferred in the implementation on #1767

For example, see an old commit in that PR here (code not in main): 5e44f62

I admit the code looks a little..strange:

let api_url = if host.contains("qa.glaredb.com") {
    "https://qa.glaredb.com/api/internal/authenticate/client"
} else {
    "https://console.glaredb.com/api/internal/authenticate/client"
};

and it's mostly to do with the asymmetry here:

   remote.glaredb.com --> console.glaredb.com
qa.remote.glaredb.com -->      qa.glaredb.com

IMO if we used remote.console.glaredb.com this would be easier to map to in a way that didn't feel a little weird.

From review we decided to pass it in as an arg, which is what we do for pgsrv and rpcsrv:

glaredb/crates/glaredb/src/args/mod.rs

Lines 40 to 83 in f1a540a

	pub struct RpcProxyArgs {
	/// TCP address to bind to.
	#[clap(short, long, value_parser, default_value_t = String::from("0.0.0.0:6444"))]
	pub bind: String,
	/// Address of the GlareDB cloud server.
	#[clap(long)]
	pub cloud_api_addr: String,
	/// Authorization code for communicating with Cloud.
	#[clap(long)]
	pub cloud_auth_code: String,
	/// Disable RPC TLS
	///
	/// (Internal)
	///
	/// Note: in the future, this will be 'on' by default
	#[clap(long, default_value="true", action = clap::ArgAction::Set, hide = true)]
	pub disable_tls: bool,
	}
	#[derive(Parser)]
	pub struct PgProxyArgs {
	/// TCP address to bind to.
	#[clap(short, long, value_parser, default_value_t = String::from("0.0.0.0:6544"))]
	pub bind: String,
	/// Path to SSL server cert to use.
	#[clap(long)]
	pub ssl_server_cert: Option<String>,
	/// Path to SSL server key to use.
	#[clap(long)]
	pub ssl_server_key: Option<String>,
	/// Address of the GlareDB cloud server.
	#[clap(long)]
	pub cloud_api_addr: String,
	/// Authorization code for communicating with Cloud.
	#[clap(long)]
	pub cloud_auth_code: String,
	}

I'd be down to revisit/rethink this to clean it up.

Appreciate the question!