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

[Python Driver] client.close() doesn't appear to work #669

Closed
TheDr1ver opened this issue Jun 28, 2024 · 2 comments
Closed

[Python Driver] client.close() doesn't appear to work #669

TheDr1ver opened this issue Jun 28, 2024 · 2 comments
Assignees

Comments

@TheDr1ver
Copy link

https://github.com/vaticle/typedb-driver/blob/6cfb17d2561f5d6f17aeecf8d14592ef22874572/python/typedb/connection/driver.py#L96

shouldn't this line be if self.is_open() instead of if not self.is_open()?

Otherwise if you run client.close() followed by client.is_open() it still shows the client as being open... effectively client.close() isn't doing anything right now as far as I can tell.

@farost
Copy link
Member

farost commented Jul 5, 2024

You're totally correct, it is a bug in our logic.

Moreover, it indicates a hole in our tests! I've created an issue for behaviour tests to prevent it from happening in the future, and I'll add an integration test for this driver with the fix.

farost added a commit that referenced this issue Jul 5, 2024
…ing for both core and cloud (#670)

## Usage and product changes
We fix the issue #669,
where the Python Driver didn't close the connection when calling
`TypeDBDriver.close()`.

## Implementation
We needed to reverse the condition of `is_open()`.

The main issue is the absence of tests for our drivers' connections, so
we:
* add new integration tests for both core and cloud;
* create an issue for behaviour tests to cover this hole for a brighter
future: typedb/typedb-behaviour#289.
@farost
Copy link
Member

farost commented Jul 5, 2024

Thanks for noticing it! The fix for this issue will be a part of the next release (hopefully by the end of the next week).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants