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

Don't create config folder on extension listing #15530

Merged

Conversation

lcostantino
Copy link
Contributor

@lcostantino lcostantino commented Jan 2, 2025

Not sure if this is needed, but whenever extensions are listed if .duckdb folder is not existent it won't be created by default.

Closes: #15471

Comment on lines 91 to 93
if (!create) {
return extension_directory;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a corner case where extension_directory exists, and the subdirectories are still created.

Possibly create should wrap all calls of CreateDirectory (2 locations).

@duckdb-draftbot duckdb-draftbot marked this pull request as draft January 2, 2025 22:37
@lcostantino lcostantino marked this pull request as ready for review January 2, 2025 23:39
@@ -120,8 +120,8 @@ class ExtensionHelper {
static ExtensionUpdateResult UpdateExtension(ClientContext &context, const string &extension_name);

//! Get the extension directory base on the current config
static string ExtensionDirectory(ClientContext &context);
static string ExtensionDirectory(DatabaseInstance &db, FileSystem &fs);
static string ExtensionDirectory(ClientContext &context, bool create = true);
Copy link
Collaborator

@Mytherin Mytherin Jan 3, 2025

Choose a reason for hiding this comment

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

For clarity can we add this functionality as a separate method - GetExtensionDirectoryPath - which only returns the path (i.e. runs the first part of the current ExtensionDirectory function)? Then ExtensionDirectory can call that method and do the creation, and we can call GetExtensionDirectoryPath directly if we do not need to create the directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

@duckdb-draftbot duckdb-draftbot marked this pull request as draft January 3, 2025 13:38
@lcostantino lcostantino marked this pull request as ready for review January 3, 2025 14:01
@duckdb-draftbot duckdb-draftbot marked this pull request as draft January 3, 2025 19:03
@lcostantino lcostantino marked this pull request as ready for review January 3, 2025 19:07
@Mytherin Mytherin merged commit 4488c61 into duckdb:main Jan 4, 2025
47 checks passed
@Mytherin
Copy link
Collaborator

Mytherin commented Jan 4, 2025

Thanks!

github-actions bot pushed a commit to duckdb/duckdb-r that referenced this pull request Jan 4, 2025
Don't create config folder on extension listing (duckdb/duckdb#15530)
[Python] Align the behavior between `sql` and `execute` for `.pl()` call (duckdb/duckdb#15537)
Update year in license file to 2025 (duckdb/duckdb#15545)
[Julia] Improves Julia support for scalar UDFs (duckdb/duckdb#15430)
github-actions bot added a commit to duckdb/duckdb-r that referenced this pull request Jan 4, 2025
Don't create config folder on extension listing (duckdb/duckdb#15530)
[Python] Align the behavior between `sql` and `execute` for `.pl()` call (duckdb/duckdb#15537)
Update year in license file to 2025 (duckdb/duckdb#15545)
[Julia] Improves Julia support for scalar UDFs (duckdb/duckdb#15430)

Co-authored-by: krlmlr <krlmlr@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

listing extensions creates $HOME/.duckdb/
3 participants