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

lock working for async recv #165

Merged
merged 11 commits into from
Mar 3, 2025
Merged

lock working for async recv #165

merged 11 commits into from
Mar 3, 2025

Conversation

maxwellflitton
Copy link
Contributor

async batching is now supported with the following unit test:

class TestAsyncWsSurrealConnection(IsolatedAsyncioTestCase):

    async def asyncSetUp(self):
        self.url = "ws://localhost:8000"
        self.password = "root"
        self.username = "root"
        self.vars_params = {
            "username": self.username,
            "password": self.password,
        }
        self.database_name = "test_db"
        self.namespace = "test_ns"
        self.data = {
            "username": self.username,
            "password": self.password,
        }
        self.connection = AsyncWsSurrealConnection(self.url)
        _ = await self.connection.signin(self.vars_params)
        _ = await self.connection.use(namespace=self.namespace, database=self.database_name)

    async def test_batch(self):
        async with asyncio.TaskGroup() as tg:
            tasks = [tg.create_task(self.connection.query("select $p**2 as ret from {}", dict(p=num))) for num in range(5)]

        outcome = [t.result()[0]["ret"] for t in tasks]
        self.assertEqual([0, 1, 4, 9, 16], outcome)
        await self.connection.socket.close()

Anton-2 and others added 5 commits February 27, 2025 12:19
CI tests are failing on this merge but this is because it's introducing a new test case into the `async-batching` branch that the `async-batching` branch needs to address
@maxwellflitton
Copy link
Contributor Author

@Anton-2 thanks for the pull request, your approach has been merged to this branch now. However, the tests are all now showing a lot of resource warnings flaky tests where some of the tests are randomly breaking. I'm going to open up another branch and just push the Lock so we get the functionality for now and then continue to work on this. Our CI runs the test suite single threaded multiple times with different python versions and different surrealDB versions. Some of these query are now randomly coming back with wrong length of results, or a certain record already being there. While your solution is more elegant and performant than a lock, we have to ensure stability first.

@maxwellflitton
Copy link
Contributor Author

maxwellflitton commented Mar 3, 2025

@Anton-2 done some jigging with the workflows and reset my system in terms of dependencies and you're code is now passing all tests with no warnings. So I'm going to merge your code into main, thanks for being patient and I really appreciate the pull request. Your commits are in this pull request so I haven't erased your contribution.

@maxwellflitton maxwellflitton merged commit 855fc95 into main Mar 3, 2025
24 checks passed
@maxwellflitton maxwellflitton deleted the async-batching branch March 3, 2025 10:47
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