Skip to content

orm: update asyncpg_blocklist#77370

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
RichardJCai:update_asyncpg_blocklist
Mar 16, 2022
Merged

orm: update asyncpg_blocklist#77370
craig[bot] merged 1 commit intocockroachdb:masterfrom
RichardJCai:update_asyncpg_blocklist

Conversation

@RichardJCai
Copy link
Copy Markdown
Contributor

Release justification: ORM test only change

Release note: None

Fixes #76490

@RichardJCai RichardJCai requested review from a team and rafiss March 3, 2022 23:10
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Collaborator

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @rafiss and @RichardJCai)


pkg/cmd/roachtest/tests/asyncpg_blocklist.go, line 27 at r1 (raw file):

// in the test log.
var asyncpgBlocklist22_1 = blocklist{
	"test_cursor.TestCursor.test_cursor_03": "unknown",

oh hm, i just noticed that these are apparently passing on 21.2. i think that means it's a regression. do you know the error message behind these?


pkg/cmd/roachtest/tests/asyncpg_blocklist.go, line 33 at r1 (raw file):

var asyncpgBlocklist21_2 = blocklist{}

var asyncpgIgnoreList22_1 = blocklist{

would you be willing to move these into the main blocklist? that way we will know if any start passing. (the ignore list is meant for flaky tests, not for tests that are expected to fail)

@RichardJCai RichardJCai force-pushed the update_asyncpg_blocklist branch from a6581f4 to a4af60c Compare March 4, 2022 19:58
Copy link
Copy Markdown
Contributor Author

@RichardJCai RichardJCai left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @rafiss)


pkg/cmd/roachtest/tests/asyncpg_blocklist.go, line 27 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

oh hm, i just noticed that these are apparently passing on 21.2. i think that means it's a regression. do you know the error message behind these?

Here's the error for test_cursor_03

Traceback (most recent call last):
  File "/mnt/data1/asyncpg/asyncpg/_testbase/__init__.py", line 92, in wrapper
    self.loop.run_until_complete(coro)
  File "/usr/lib/python3.8/asyncio/base_events.py", line 616, in run_until_complete
    return future.result()
  File "/mnt/data1/asyncpg/tests/test_cursor.py", line 142, in test_cursor_03
    async with self.con.transaction():
  File "/mnt/data1/asyncpg/asyncpg/transaction.py", line 62, in __aenter__
    await self.start()
  File "/mnt/data1/asyncpg/asyncpg/transaction.py", line 138, in start
    await self._connection.execute(query)
  File "/mnt/data1/asyncpg/asyncpg/connection.py", line 315, in execute
    return await self._protocol.query(query, timeout)
  File "asyncpg/protocol/protocol.pyx", line 338, in query
    return await waiter
asyncpg.exceptions._base.UnknownPostgresError: there is already a transaction in progress

And test_pool_11

ERROR: test_pool_11 (test_pool.TestPool)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/data1/asyncpg/asyncpg/_testbase/__init__.py", line 92, in wrapper
    self.loop.run_until_complete(coro)
  File "/usr/lib/python3.8/asyncio/base_events.py", line 616, in run_until_complete
    return future.result()
  File "/mnt/data1/asyncpg/tests/test_pool.py", line 210, in test_pool_11
    ps_cur = await ps.cursor()
  File "/mnt/data1/asyncpg/asyncpg/pool.py", line 959, in __aexit__
    await self.pool.release(con)
  File "/mnt/data1/asyncpg/asyncpg/pool.py", line 833, in release
    return await asyncio.shield(ch.release(timeout))
  File "/mnt/data1/asyncpg/asyncpg/pool.py", line 218, in release
    raise ex
  File "/mnt/data1/asyncpg/asyncpg/pool.py", line 208, in release
    await self._con.reset(timeout=budget)
  File "/mnt/data1/asyncpg/asyncpg/connection.py", line 1347, in reset
    await self.execute(reset_query, timeout=timeout)
  File "/mnt/data1/asyncpg/asyncpg/connection.py", line 315, in execute
    return await self._protocol.query(query, timeout)
  File "asyncpg/protocol/protocol.pyx", line 338, in query
    return await waiter
asyncpg.exceptions._base.UnknownPostgresError: there is no transaction in progress

Not helpful right now without digging in more but it seems like we do have a regression.

test_cursor_02 and test_cursor_04 were already failing before, interesting that test_cursor_03 was okay before, what was the status of cursor support before this?


pkg/cmd/roachtest/tests/asyncpg_blocklist.go, line 33 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

would you be willing to move these into the main blocklist? that way we will know if any start passing. (the ignore list is meant for flaky tests, not for tests that are expected to fail)

Done.

@RichardJCai RichardJCai requested a review from rafiss March 7, 2022 19:48
Copy link
Copy Markdown
Collaborator

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @rafiss and @RichardJCai)


pkg/cmd/roachtest/tests/asyncpg_blocklist.go, line 27 at r1 (raw file):

Previously, RichardJCai (Richard Cai) wrote…

Here's the error for test_cursor_03

Traceback (most recent call last):
  File "/mnt/data1/asyncpg/asyncpg/_testbase/__init__.py", line 92, in wrapper
    self.loop.run_until_complete(coro)
  File "/usr/lib/python3.8/asyncio/base_events.py", line 616, in run_until_complete
    return future.result()
  File "/mnt/data1/asyncpg/tests/test_cursor.py", line 142, in test_cursor_03
    async with self.con.transaction():
  File "/mnt/data1/asyncpg/asyncpg/transaction.py", line 62, in __aenter__
    await self.start()
  File "/mnt/data1/asyncpg/asyncpg/transaction.py", line 138, in start
    await self._connection.execute(query)
  File "/mnt/data1/asyncpg/asyncpg/connection.py", line 315, in execute
    return await self._protocol.query(query, timeout)
  File "asyncpg/protocol/protocol.pyx", line 338, in query
    return await waiter
asyncpg.exceptions._base.UnknownPostgresError: there is already a transaction in progress

And test_pool_11

ERROR: test_pool_11 (test_pool.TestPool)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/data1/asyncpg/asyncpg/_testbase/__init__.py", line 92, in wrapper
    self.loop.run_until_complete(coro)
  File "/usr/lib/python3.8/asyncio/base_events.py", line 616, in run_until_complete
    return future.result()
  File "/mnt/data1/asyncpg/tests/test_pool.py", line 210, in test_pool_11
    ps_cur = await ps.cursor()
  File "/mnt/data1/asyncpg/asyncpg/pool.py", line 959, in __aexit__
    await self.pool.release(con)
  File "/mnt/data1/asyncpg/asyncpg/pool.py", line 833, in release
    return await asyncio.shield(ch.release(timeout))
  File "/mnt/data1/asyncpg/asyncpg/pool.py", line 218, in release
    raise ex
  File "/mnt/data1/asyncpg/asyncpg/pool.py", line 208, in release
    await self._con.reset(timeout=budget)
  File "/mnt/data1/asyncpg/asyncpg/connection.py", line 1347, in reset
    await self.execute(reset_query, timeout=timeout)
  File "/mnt/data1/asyncpg/asyncpg/connection.py", line 315, in execute
    return await self._protocol.query(query, timeout)
  File "asyncpg/protocol/protocol.pyx", line 338, in query
    return await waiter
asyncpg.exceptions._base.UnknownPostgresError: there is no transaction in progress

Not helpful right now without digging in more but it seems like we do have a regression.

test_cursor_02 and test_cursor_04 were already failing before, interesting that test_cursor_03 was okay before, what was the status of cursor support before this?

"cursor" has a few different meanings. is this referring to DECLARE cursor? that was only added in v22.1 #74006


pkg/cmd/roachtest/tests/asyncpg_blocklist.go, line 33 at r1 (raw file):

Previously, RichardJCai (Richard Cai) wrote…

Done.

sorry - i meant that all of these tests should be in asyncpgBlocklist, and nothing should be in asyncpgIgnoreList

@RichardJCai RichardJCai force-pushed the update_asyncpg_blocklist branch from a4af60c to 4c14db8 Compare March 9, 2022 18:32
@RichardJCai RichardJCai requested a review from rafiss March 9, 2022 18:33
Copy link
Copy Markdown
Contributor Author

@RichardJCai RichardJCai left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @rafiss and @RichardJCai)


pkg/cmd/roachtest/tests/asyncpg_blocklist.go, line 33 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

sorry - i meant that all of these tests should be in asyncpgBlocklist, and nothing should be in asyncpgIgnoreList

Done.

Copy link
Copy Markdown
Collaborator

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

lgtm!

i found that my change in #76834 actually fixes test_cursor_03 and test_pool_11.

i do want to merge that PR, so if you want, you could remove those two tests from the blocklist

Release justification: ORM test only change

Release note: None
@rafiss rafiss force-pushed the update_asyncpg_blocklist branch from 4c14db8 to 62f6339 Compare March 16, 2022 15:51
@rafiss
Copy link
Copy Markdown
Collaborator

rafiss commented Mar 16, 2022

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Mar 16, 2022

Build succeeded:

@craig craig bot merged commit 4b75fd0 into cockroachdb:master Mar 16, 2022
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.

roachtest: asyncpg failed

3 participants