Skip to content

Conversation

Tishj
Copy link
Contributor

@Tishj Tishj commented Aug 15, 2024

This method allows the user to change the connection that's used by the module internally when invoking DuckDBPyConnection methods (such as .sql, .table etc..) directly on the duckdb module.

I also considered adding a context manager for this, to basically create a scope where the default_connection is overridden and reset back after the scope ends, that might be a future PR.

Incompatible change:

Hopefully nobody cares but I figured I'd mention it regardless, default_connection is no longer an attribute on the module, instead it's now a method.
It doesn't seem to be possible to create a property that can have getters and setters on the module so turning it into default_connection() and set_default_connection(connection: DuckDBPyConnection) was the next best thing

@Mytherin
Copy link
Collaborator

Thanks for the PR!

I don't quite follow what this PR enables - can you not already assign the default connection like so?

import duckdb
duckdb.default_connection = duckdb.connect()

If that does not work, could we not make that work instead of changing the API?

@Tishj
Copy link
Contributor Author

Tishj commented Aug 16, 2024

Thanks for the PR!

I don't quite follow what this PR enables - can you not already assign the default connection like so?

import duckdb
duckdb.default_connection = duckdb.connect()

If that does not work, could we not make that work instead of changing the API?

It doesn't work, and we can't make that work.
default_connection is an attribute which is assigned the value of DuckDBPyConnection::DefaultConnection() at the import of the module

If a new value is assigned to that, it does not reflect on the internals of the module

I also explained this in the Incompatible change section of the description

@Tishj
Copy link
Contributor Author

Tishj commented Aug 16, 2024

There might be some way we can look up the current value of the default_connection attribute, but that would need to go through Python every time, and we wouldn't be able to protect it against arbitrary object assignation

@Tishj
Copy link
Contributor Author

Tishj commented Aug 16, 2024

I would also vote for removing duckdb.default_connection entirely in favor of duckdb.connect(':default:')
Perhaps set_default_connection could be DuckDBPyConnection::set_as_default instead?

@Mytherin Mytherin changed the base branch from main to feature September 25, 2024 13:29
@Mytherin Mytherin merged commit 403f944 into duckdb:feature Sep 25, 2024
15 checks passed
@Mytherin
Copy link
Collaborator

Thanks!

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.

2 participants