Skip to content

Add optional/required query_single* methods + rename retrying_transaction + update pool to create_client API#249

Merged
jaclarke merged 7 commits intomasterfrom
optional-query-single
Nov 10, 2021
Merged

Add optional/required query_single* methods + rename retrying_transaction + update pool to create_client API#249
jaclarke merged 7 commits intomasterfrom
optional-query-single

Conversation

@jaclarke
Copy link
Member

@jaclarke jaclarke commented Nov 3, 2021

No description provided.

elprans
elprans reviewed Nov 3, 2021
View reviewed changes
tests/test_async_query.py Outdated
# self.assertEqual(
# await self.con.query_single('START TRANSACTION'), None)

# with self.assertRaisesRegex(
Copy link
Member

@elprans elprans Nov 3, 2021

Choose a reason for hiding this comment

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

uncomment?

Copy link
Member Author

@jaclarke jaclarke Nov 3, 2021

Choose a reason for hiding this comment

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

I left this bit commented out for now, because of this bug: geldata/gel#3120

Copy link
Member

@elprans elprans Nov 9, 2021

Choose a reason for hiding this comment

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

Let's reference this but in the comment then.

@elprans elprans requested a review from 1st1 November 4, 2021 16:59
@jaclarke jaclarke mentioned this pull request Nov 8, 2021
Merged
@jaclarke jaclarke force-pushed the optional-query-single branch from c765073 to 5df9c6c Compare November 8, 2021 22:55
@colinhacks colinhacks mentioned this pull request Nov 9, 2021
Merged
@1st1
Copy link
Member

1st1 commented Nov 9, 2021

The connect() function should be refactored into this (in pseudo-code):

async def connect():
    warnings.warn('deprecated')
    client = create_client(concurrency=1)
    await client.ensure_connected()
    return client

Let's keep raw (unwrapped) connection around and add a top-level function async connect_raw() to return it. But let's not document it just yet.

@1st1
Copy link
Member

1st1 commented Nov 10, 2021

@jaclarke James, everything looks great. Can you fix the CI issue and merge?

This can go in a separate PR #249 (comment)

@jaclarke jaclarke force-pushed the optional-query-single branch from 9109925 to d975c99 Compare November 10, 2021 11:44
@jaclarke jaclarke merged commit f2ae0d0 into master Nov 10, 2021
@jaclarke jaclarke deleted the optional-query-single branch November 10, 2021 12:18
@jaclarke jaclarke mentioned this pull request Nov 11, 2021
Merged
elprans added a commit that referenced this pull request Nov 12, 2021
@elprans
edgedb-python v0.18.0
7f96277 
Changes
=======

* support protocol version 0.12
  (by @fmoor in 5ce52ed)

* Update to protocol version 0.13
  (by @1st1 in c690250, 8by @fmoor in 5ce52ed, by @jaclarke in a5a6218 for #253)

* Lookup edgedb.toml recursively in parent directories
  (by @fmoor in d9a12b8 for #245)

* Update connection parameter resolution
  (by @jaclarke in 085f574 for #241)

* Implement EDGEDB_CLIENT_SECURITY
  (by @fmoor in ac46c37)

* Add optional/required `query_single*` methods + rename `retrying_transaction` + update pool to `create_client` API
  (by @jaclarke in f2ae0d0 for #249)

Deprecations
============

* Deprecate Pool.acquire() and Pool.release()
  (by @fmoor in 2d501e9 for #217)

* Rename tls_verify_hostname to tls_security
  (by @fmoor in 2086b86)

Fixes
=====

* Do not attempt to del transport in connection_lost if it has been already
  (by @elprans in c719e79 for #215)

* Retry if start() raises a retryable error (#228)
  (by @fantix in ffaae01 for #228)

* Fix broken pool connection cleanup
  (by @fantix in be44959 for #222)

* Fix deprecated usage of SSLContext (#231)
  (by @elprans in f73f999 for #231)

* Bugfix: _borrowed_for is now set entering a transaction (#233)
  (by @fantix in bf763d1 for #233)

* Send zero arguments as zero-length bytes in proto 0.12 (#238)
  (by @tailhook in f51dd51 for #238)

* Fix retrying_transaction() on network errors
  (by @fmoor in cc001e6)

* Fix connection and retry options on AsyncIOPool (#237)
  (by @tailhook in 44e279f for #237)

* Auto retry read-only queries outside transactions (#243)
  (by @fmoor in 76bb586 for #243)

* Fix `credentials_file` argument typo on `async_connect` (#252)
  (by @mkniewallner in b21b70a for #252)
elprans added a commit that referenced this pull request Nov 12, 2021
@elprans
edgedb-python v0.18.0
2272824 
Changes
=======

* support protocol version 0.12
  (by @fmoor in 5ce52ed)

* Update to protocol version 0.13
  (by @1st1 in c690250, 8by @fmoor in 5ce52ed, by @jaclarke in a5a6218 for #253)

* Lookup `edgedb.toml` recursively in parent directories
  (by @fmoor in d9a12b8 for #245)

* Update connection parameter resolution
  (by @jaclarke in 085f574 for #241)

* Implement `EDGEDB_CLIENT_SECURITY`
  (by @fmoor in ac46c37)

* Add optional/required `query_single*` methods + rename `retrying_transaction` + update pool to `create_client` API
  (by @jaclarke in f2ae0d0 for #249)

Deprecations
============

* Deprecate `Pool.acquire()` and `Pool.release()`
  (by @fmoor in 2d501e9 for #217)

* Rename `tls_verify_hostname` to `tls_security`
  (by @fmoor in 2086b86)

Fixes
=====

* Do not attempt to del transport in `connection_lost` if it has been already
  (by @elprans in c719e79 for #215)

* Retry if `start()` raises a retryable error (#228)
  (by @fantix in ffaae01 for #228)

* Fix broken pool connection cleanup
  (by @fantix in be44959 for #222)

* Fix deprecated usage of `SSLContext` (#231)
  (by @elprans in f73f999 for #231)

* Bugfix: `_borrowed_for` is now set entering a transaction (#233)
  (by @fantix in bf763d1 for #233)

* Send zero arguments as zero-length bytes in proto 0.12 (#238)
  (by @tailhook in f51dd51 for #238)

* Fix `retrying_transaction()` on network errors
  (by @fmoor in cc001e6)

* Fix connection and retry options on `AsyncIOPool` (#237)
  (by @tailhook in 44e279f for #237)

* Auto retry read-only queries outside transactions (#243)
  (by @fmoor in 76bb586 for #243)

* Fix `credentials_file` argument typo on `async_connect` (#252)
  (by @mkniewallner in b21b70a for #252)
@elprans elprans mentioned this pull request Nov 12, 2021
Merged
elprans added a commit that referenced this pull request Nov 12, 2021
@elprans
edgedb-python v0.18.0
a854632 
Changes
=======

* support protocol version 0.12
  (by @fmoor in 5ce52ed)

* Update to protocol version 0.13
  (by @1st1 in c690250, 8by @fmoor in 5ce52ed, by @jaclarke in a5a6218 for #253)

* Lookup `edgedb.toml` recursively in parent directories
  (by @fmoor in d9a12b8 for #245)

* Update connection parameter resolution
  (by @jaclarke in 085f574 for #241)

* Implement `EDGEDB_CLIENT_SECURITY`
  (by @fmoor in ac46c37)

* Add optional/required `query_single*` methods + rename `retrying_transaction` + update pool to `create_client` API
  (by @jaclarke in f2ae0d0 for #249)

Deprecations
============

* Deprecate `Pool.acquire()` and `Pool.release()`
  (by @fmoor in 2d501e9 for #217)

* Rename `tls_verify_hostname` to `tls_security`
  (by @fmoor in 2086b86)

Fixes
=====

* Do not attempt to del transport in `connection_lost` if it has been already
  (by @elprans in c719e79 for #215)

* Retry if `start()` raises a retryable error (#228)
  (by @fantix in ffaae01 for #228)

* Fix broken pool connection cleanup
  (by @fantix in be44959 for #222)

* Fix deprecated usage of `SSLContext` (#231)
  (by @elprans in f73f999 for #231)

* Bugfix: `_borrowed_for` is now set entering a transaction (#233)
  (by @fantix in bf763d1 for #233)

* Send zero arguments as zero-length bytes in proto 0.12 (#238)
  (by @tailhook in f51dd51 for #238)

* Fix `retrying_transaction()` on network errors
  (by @fmoor in cc001e6)

* Fix connection and retry options on `AsyncIOPool` (#237)
  (by @tailhook in 44e279f for #237)

* Auto retry read-only queries outside transactions (#243)
  (by @fmoor in 76bb586 for #243)

* Fix `credentials_file` argument typo on `async_connect` (#252)
  (by @mkniewallner in b21b70a for #252)
elprans added a commit that referenced this pull request Nov 12, 2021
@elprans
edgedb-python v0.18.0
36efb5f 
Changes
=======

* support protocol version 0.12
  (by @fmoor in 5ce52ed)

* Update to protocol version 0.13
  (by @1st1 in c690250, 8by @fmoor in 5ce52ed, by @jaclarke in a5a6218 for #253)

* Lookup `edgedb.toml` recursively in parent directories
  (by @fmoor in d9a12b8 for #245)

* Update connection parameter resolution
  (by @jaclarke in 085f574 for #241)

* Implement `EDGEDB_CLIENT_SECURITY`
  (by @fmoor in ac46c37)

* Add optional/required `query_single*` methods + rename `retrying_transaction` + update pool to `create_client` API
  (by @jaclarke in f2ae0d0 for #249)

Deprecations
============

* Deprecate `Pool.acquire()` and `Pool.release()`
  (by @fmoor in 2d501e9 for #217)

* Rename `tls_verify_hostname` to `tls_security`
  (by @fmoor in 2086b86)

Fixes
=====

* Do not attempt to del transport in `connection_lost` if it has been already
  (by @elprans in c719e79 for #215)

* Retry if `start()` raises a retryable error (#228)
  (by @fantix in ffaae01 for #228)

* Fix broken pool connection cleanup
  (by @fantix in be44959 for #222)

* Fix deprecated usage of `SSLContext` (#231)
  (by @elprans in f73f999 for #231)

* Bugfix: `_borrowed_for` is now set entering a transaction (#233)
  (by @fantix in bf763d1 for #233)

* Send zero arguments as zero-length bytes in proto 0.12 (#238)
  (by @tailhook in f51dd51 for #238)

* Fix `retrying_transaction()` on network errors
  (by @fmoor in cc001e6)

* Fix connection and retry options on `AsyncIOPool` (#237)
  (by @tailhook in 44e279f for #237)

* Auto retry read-only queries outside transactions (#243)
  (by @fmoor in 76bb586 for #243)

* Fix `credentials_file` argument typo on `async_connect` (#252)
  (by @mkniewallner in b21b70a for #252)
fantix
fantix reviewed Jan 6, 2022
View reviewed changes
edgedb/asyncio_pool.py

async def ensure_connected(self):
for ch in self._impl._holders:
if ch._con is not None and ch._con.is_closed():
Copy link
Member

@fantix fantix Jan 6, 2022

Choose a reason for hiding this comment

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

Just to be sure - this is not intentionally and ch._con.is_closed() here for some reason, right? Because I think it's a typo, it should be and not ch._con.is_closed().

Copy link
Member

@elprans elprans Jan 7, 2022

Choose a reason for hiding this comment

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

Yea, it's a typo

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

Reviewers

@elprans elprans elprans approved these changes

@fantix fantix fantix left review comments

@1st1 1st1 1st1 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jaclarke @1st1 @elprans @fantix