Skip to content
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

Query remote/cloud instances for extensions. #1438

Merged
merged 3 commits into from
Jan 13, 2025
Merged

Conversation

dnwpark
Copy link
Contributor

@dnwpark dnwpark commented Jan 10, 2025

Adds support to egedb extension list for remote and cloud instances.

Close #1437


Running with a remote dev instance produced:

┌─────────────┬─────────┐
│ Name        │ Version │
├─────────────┼─────────┤
│ ai          │ 1.0     │
│ graphql     │ 1.0     │
│ pgvector    │ 0.7     │
│ edgeql_http │ 1.0     │
│ pg_unaccent │ 1.1     │
│ pg_trgm     │ 1.6     │
│ pgcrypto    │ 1.3     │
│ postgis     │ 3.4     │
│ _conf       │ 1.0     │
│ auth        │ 1.0     │
└─────────────┴─────────┘

Running with a cloud instance produced:

┌─────────────┬─────────┐
│ Name        │ Version │
├─────────────┼─────────┤
│ pgvector    │ 0.5     │
│ ai          │ 1.0     │
│ graphql     │ 1.0     │
│ edgeql_http │ 1.0     │
│ pg_trgm     │ 1.6     │
│ pgcrypto    │ 1.3     │
│ auth        │ 1.0     │
└─────────────┴─────────┘

@dnwpark dnwpark requested a review from mmastrac January 10, 2025 23:46
@@ -53,27 +55,65 @@ fn get_local_instance(options: &Options) -> Result<InstanceInfo, anyhow::Error>
Ok(inst)
}

#[tokio::main(flavor = "current_thread")]
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems out of place...

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see, this is being used to convert the function to a blocking one. I didn't know you could do this. It's not ideal -- I would recommend creating yourself a new tokio runtime and using block_on (see

fn block_on<T>(f: impl Future<Output = anyhow::Result<T>>) -> anyhow::Result<T> {
)

.unwrap_or_default();
table.add_row(row![name, version]);
}
table.set_titles(row!["Name", "Version"]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice fix!

let extension_loader = inst.extension_loader_path()?;
let output = run_extension_loader(&extension_loader, Some("--list-packages"), None::<&str>)?;
let value: serde_json::Value = serde_json::from_str(&output)?;
let extensions: Vec<ExtensionInfo> = (|| {
Copy link
Contributor

Choose a reason for hiding this comment

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

Generally it would be better to create a new function rather than define a closure and immediately execute it.

@dnwpark dnwpark force-pushed the extension-list-remote branch from cee441b to e4ecf05 Compare January 11, 2025 00:38
@dnwpark dnwpark force-pushed the extension-list-remote branch from e4ecf05 to 70499c6 Compare January 11, 2025 00:44
Copy link
Contributor

@mmastrac mmastrac left a comment

Choose a reason for hiding this comment

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

LGTM!

@dnwpark dnwpark merged commit e77ba9e into master Jan 13, 2025
16 of 17 checks passed
@dnwpark dnwpark deleted the extension-list-remote branch January 13, 2025 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make the CLI handle preinstalled extension packages on remote instances
2 participants