async iterating over nodes is not working

[AnikinNN]

Expected Behavior (Mandatory)

It is possible to iterate over nodes in async style

Actual Behavior (Mandatory)

It is not possible to iterate over nodes in async style. You have to fetch all nodes to memory before iteration

How to Reproduce the Problem

Run code below

Simple Example

import asyncio

import neomodel
import neomodel.config
from neomodel import AsyncStructuredNode, IntegerProperty

neomodel.config.DATABASE_URL = 'bolt://neo4j:12345678@neo4j:7687'

class SomeNode(AsyncStructuredNode):
    number = IntegerProperty(required=True)

async def delete():
    nodes = await SomeNode.nodes
    for node in nodes:
        await node.delete()

async def main():
    # clean up
    await delete()

    for i in range(10):
        await SomeNode(number=i).save()

    nodes = await SomeNode.nodes

    assert isinstance(nodes, list)
    assert all(isinstance(i, SomeNode) for i in nodes)

    try:
        for i in SomeNode.nodes:
            # make some iteration
            print(i)
    except Exception as e:
        # SomeNode.nodes is not sync iterable as expected
        print(e)

    try:
        async for node in SomeNode.nodes:
            print(node)
    except Exception as e:
        # SomeNode.nodes has __aiter__() method
        # but it has a bug at
        # neomodel.async_.match line 813
        print(e)

    # this works fine but this approach is not async as it fetches all nodes before iterating
    for node in await SomeNode.nodes:
        print(node)

if __name__ == '__main__':
    asyncio.run(main())

Specifications (Mandatory)

don't know what I should write here

Currently used versions

Versions

• OS: linux, 3.10.14-slim
• Library:5.3.0 [extras]
• Neo4j:5.19.0 community

[mariusconjeaud]

@AnikinNN Please check the corresponding PR !

[AnikinNN]

Great work!

I see two points which I should mention:

• this code still loads all nodes to memory before iteration. However I suppose this should not be done here as it requires some kind of pagination. Pagination should not be hided under the hood as it can bring some problems with synchronization
• the tests won't show this error because actual errors wrapped into try/catch

I can propose better test:

@mark_async_test
async def test_async_iterator():
    n = 10
    if AsyncUtil.is_async_code:
        for i in range(n):
            await Coffee(name=f"xxx_{i}", price=i).save()

        nodes = await Coffee.nodes
        # assert that nodes was created
        assert isinstance(nodes, list)
        assert all(isinstance(i, Coffee) for i in nodes)
        assert len(nodes) == n

        counter = 0
        async for node in Coffee.nodes:
            assert isinstance(node, Coffee)
            counter += 1

        # assert that generator runs loop above
        assert counter == n

[mariusconjeaud]

Yes indeed, I agree with your first point ! To be honest, I focussed here on fixing the __aiter__ bug ; making it truly async would have to introduce a breaking change by introducing pagination I guess, and I did not want to go into it right now. But if you have suggestions regarding this, then I'm very open to them !

As for the test, thank you, I updated it. Re-reading my test, I see it had no assertions and had the try-except still, I don't know what I had in mind 😄 . I guess I was tired from going down into the aiter and ast._execute and yield mechanism 🤣

[AnikinNN]

making it truly async would have to introduce a breaking change by introducing pagination I guess

Definitely agree with you

[mariusconjeaud]

OK, so let me close this and include it in 5.3.1 ; and then we can open an issue for improving this behaviour.