Skip to content

fix: do not attempt db connection if jsonc section is missing #375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Apr 25, 2025
4 changes: 0 additions & 4 deletions crates/pgt_cli/src/cli_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ pub struct CliOptions {
#[bpaf(long("use-server"), switch, fallback(false))]
pub use_server: bool,

/// Skip connecting to the database and only run checks that don't require a database connection.
#[bpaf(long("skip-db"), switch, fallback(false))]
pub skip_db: bool,

/// Print additional diagnostics, and some diagnostics show more information. Also, print out what files were processed and which ones were modified.
#[bpaf(long("verbose"), switch, fallback(false))]
pub verbose: bool,
Expand Down
1 change: 0 additions & 1 deletion crates/pgt_cli/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,6 @@ pub(crate) trait CommandRunner: Sized {
configuration,
vcs_base_path,
gitignore_matches,
skip_db: cli_options.skip_db,
})?;

let execution = self.get_execution(cli_options, console, workspace)?;
Expand Down
8 changes: 8 additions & 0 deletions crates/pgt_configuration/src/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ use serde::{Deserialize, Serialize};
#[partial(serde(rename_all = "camelCase", default, deny_unknown_fields))]
pub struct DatabaseConfiguration {
/// The host of the database.
/// Required if you want database-related features.
/// All else falls back to sensible defaults.
#[partial(bpaf(long("host")))]
pub host: String,

Expand All @@ -35,11 +37,17 @@ pub struct DatabaseConfiguration {
/// The connection timeout in seconds.
#[partial(bpaf(long("conn_timeout_secs"), fallback(Some(10)), debug_fallback))]
pub conn_timeout_secs: u16,

/// Actively disable all database-related features.
#[partial(bpaf(long("disable-db"), switch, fallback(Some(false))))]
#[partial(cfg_attr(feature = "schema", schemars(skip)))]
pub disable_connection: bool,
}

impl Default for DatabaseConfiguration {
fn default() -> Self {
Self {
disable_connection: false,
host: "127.0.0.1".to_string(),
port: 5432,
username: "postgres".to_string(),
Expand Down
3 changes: 2 additions & 1 deletion crates/pgt_configuration/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,9 @@ impl PartialConfiguration {
username: Some("postgres".to_string()),
password: Some("postgres".to_string()),
database: Some("postgres".to_string()),
conn_timeout_secs: Some(10),
allow_statement_executions_against: Default::default(),
conn_timeout_secs: Some(10),
disable_connection: Some(false),
}),
}
}
Expand Down
1 change: 0 additions & 1 deletion crates/pgt_lsp/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,6 @@ impl Session {
configuration: fs_configuration,
vcs_base_path,
gitignore_matches,
skip_db: false,
});

if let Err(error) = result {
Expand Down
5 changes: 3 additions & 2 deletions crates/pgt_lsp/tests/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -773,14 +773,15 @@ async fn test_execute_statement() -> Result<()> {
.to_string();
let host = test_db.connect_options().get_host().to_string();

let conf = PartialConfiguration {
let mut conf = PartialConfiguration::init();
conf.merge_with(PartialConfiguration {
db: Some(PartialDatabaseConfiguration {
database: Some(database),
host: Some(host),
..Default::default()
}),
..Default::default()
};
});

fs.insert(
url!("postgrestools.jsonc").to_file_path().unwrap(),
Expand Down
11 changes: 11 additions & 0 deletions crates/pgt_workspace/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ impl Default for LinterSettings {
/// Database settings for the entire workspace
#[derive(Debug)]
pub struct DatabaseSettings {
pub enable_connection: bool,
pub host: String,
pub port: u16,
pub username: String,
Expand All @@ -280,6 +281,7 @@ pub struct DatabaseSettings {
impl Default for DatabaseSettings {
fn default() -> Self {
Self {
enable_connection: false,
host: "127.0.0.1".to_string(),
port: 5432,
username: "postgres".to_string(),
Expand All @@ -295,6 +297,13 @@ impl From<PartialDatabaseConfiguration> for DatabaseSettings {
fn from(value: PartialDatabaseConfiguration) -> Self {
let d = DatabaseSettings::default();

// "host" is the minimum required setting for database features
// to be enabled.
let enable_connection = value
.host
.as_ref()
.is_some_and(|_| value.disable_connection.is_none_or(|disabled| !disabled));

let database = value.database.unwrap_or(d.database);
let host = value.host.unwrap_or(d.host);

Expand All @@ -312,6 +321,8 @@ impl From<PartialDatabaseConfiguration> for DatabaseSettings {
.unwrap_or(false);

Self {
enable_connection,

port: value.port.unwrap_or(d.port),
username: value.username.unwrap_or(d.username),
password: value.password.unwrap_or(d.password),
Expand Down
1 change: 0 additions & 1 deletion crates/pgt_workspace/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ pub struct UpdateSettingsParams {
pub vcs_base_path: Option<PathBuf>,
pub gitignore_matches: Vec<String>,
pub workspace_directory: Option<PathBuf>,
pub skip_db: bool,
}

#[derive(Debug, serde::Serialize, serde::Deserialize)]
Expand Down
10 changes: 4 additions & 6 deletions crates/pgt_workspace/src/workspace/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,10 @@ impl Workspace for WorkspaceServer {

tracing::info!("Updated settings in workspace");

if !params.skip_db {
self.connection
.write()
.unwrap()
.set_conn_settings(&self.settings().as_ref().db);
}
self.connection
.write()
.unwrap()
.set_conn_settings(&self.settings().as_ref().db);

tracing::info!("Updated Db connection settings");

Expand Down
5 changes: 5 additions & 0 deletions crates/pgt_workspace/src/workspace/server/db_connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ impl DbConnection {
}

pub(crate) fn set_conn_settings(&mut self, settings: &DatabaseSettings) {
if !settings.enable_connection {
tracing::info!("Database connection disabled.");
return;
}

let config = PgConnectOptions::new()
.host(&settings.host)
.port(settings.port)
Expand Down
2 changes: 1 addition & 1 deletion docs/schemas/0.0.0/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@
]
},
"host": {
"description": "The host of the database.",
"description": "The host of the database. Required if you want database-related features. All else falls back to sensible defaults.",
"type": [
"string",
"null"
Expand Down
2 changes: 1 addition & 1 deletion docs/schemas/latest/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@
]
},
"host": {
"description": "The host of the database.",
"description": "The host of the database. Required if you want database-related features. All else falls back to sensible defaults.",
"type": [
"string",
"null"
Expand Down
2 changes: 2 additions & 0 deletions justfile
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ serve-docs:
uv run mkdocs serve

# When you finished coding, run this command. Note that you should have already committed your changes.
# If you haven't run `sqlx prepare` at least once, you need to run `docker compose up`
# to lint the queries.
ready:
git diff --exit-code --quiet
cargo run -p xtask_codegen -- configuration
Expand Down
27 changes: 21 additions & 6 deletions packages/@postgrestools/backend-jsonrpc/src/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,21 +179,36 @@ export interface GetCompletionsParams {
*/
position: TextSize;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

auto-generated

}
export interface CompletionResult {
export interface CompletionsResult {
items: CompletionItem[];
}
export interface CompletionItem {
completion_text?: CompletionText;
description: string;
kind: CompletionItemKind;
label: string;
preselected: boolean;
score: number;
/**
* String used for sorting by LSP clients.
*/
sort_text: string;
}
/**
* The text that the editor should fill in. If `None`, the `label` should be used. Tables, for example, might have different completion_texts:

label: "users", description: "Schema: auth", completion_text: "auth.users".
*/
export interface CompletionText {
/**
* A `range` is required because some editors replace the current token, others naively insert the text. Having a range where start == end makes it an insertion.
*/
range: TextRange;
text: string;
}
export type CompletionItemKind = "table" | "function" | "column";
export type CompletionItemKind = "table" | "function" | "column" | "schema";
export interface UpdateSettingsParams {
configuration: PartialConfiguration;
gitignore_matches: string[];
skip_db: boolean;
vcs_base_path?: string;
workspace_directory?: string;
}
Expand Down Expand Up @@ -240,7 +255,7 @@ export interface PartialDatabaseConfiguration {
*/
database?: string;
/**
* The host of the database.
* The host of the database. Required if you want database-related features. All else falls back to sensible defaults.
*/
host?: string;
/**
Expand Down Expand Up @@ -414,7 +429,7 @@ export interface Workspace {
pullDiagnostics(
params: PullDiagnosticsParams,
): Promise<PullDiagnosticsResult>;
getCompletions(params: GetCompletionsParams): Promise<CompletionResult>;
getCompletions(params: GetCompletionsParams): Promise<CompletionsResult>;
updateSettings(params: UpdateSettingsParams): Promise<void>;
openFile(params: OpenFileParams): Promise<void>;
changeFile(params: ChangeFileParams): Promise<void>;
Expand Down