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

Add list(as_pages=True) for returning resource version #84

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

XeCycle
Copy link
Contributor

@XeCycle XeCycle commented Dec 24, 2024

Sorry I had some busy time and finally back to continue on #77.

Reasons for my implementation choice:

  • you said having the resourceVersion from list can be useful for other cases, I agree;
  • but working with heterogeneous iterators is too hard for typing;
  • I also considered having the iterators "return" the resourceVersion, i.e. StopIteration(version), but it is also hard to use;
  • I don't know how to map from Pod to PodList in the static typing rules...

So here it is, a bool parameter to yield each chunk as tuple (resourceVersion, items). Kubernetes docs says the version should remain constant, so users should be able to choose an arbitrary one, assuming the server well-behaves.

@gtsystem
Copy link
Owner

Hi,
I think I managed to make a generic iterator that can be used as today and can also return the version..

Here is a full example:

import asyncio
from typing import AsyncIterator, TypeVar, Generic, Iterator, Type
from lightkube.core.client import NamespacedResource
from lightkube.resources.core_v1 import Pod
from lightkube.models.meta_v1 import ObjectMeta

T = TypeVar('T')

class GenericIterator(Generic[T]):
    version: int

    def __init__(self, version, it: Iterator[T]):
        self.version = version
        self._it = it

    def __iter__(self) -> Iterator[T]:
        return self._it

def list(res: Type[NamespacedResource]) -> GenericIterator[NamespacedResource]:
    """Simulate client.list()"""
    return GenericIterator(3, (res(metadata=ObjectMeta(name=f"name-{v}")) for v in range(3)))

def sync_main():
    pods = list(Pod)
    print(pods.version)
    for pod in pods:
        print(pod.metadata.name)

print("Sync Main")
sync_main()

And respective async example:

class GenericAsyncIterator(Generic[T]):
    version: int

    def __init__(self, version, it: AsyncIterator[T]):
        self.version = version
        self._it = it

    def __aiter__(self) -> AsyncIterator[T]:
        return self._it

def async_list(res: Type[NamespacedResource]) -> GenericAsyncIterator[NamespacedResource]:
    """Simulate asyncclient.list()"""
    async def itarator() -> AsyncIterator[NamespacedResource]:
        async for v in async_range(3):
            yield res(metadata=ObjectMeta(name=f"name-{v}"))
    return GenericAsyncIterator(3, itarator())


async def async_range(count):
    for i in range(count):
        yield(i)
        await asyncio.sleep(0.0)

async def async_main():
    pods = async_list(Pod)
    print(pods.version)
    async for pod in pods:
        print(pod.metadata.name)

print("ASync Main")
loop = asyncio.run(async_main())

Both PyCharm and VSCode seems to be able to autocomplete just fine. Would you like to explore this direction?
The solution proposed in this PR add a parameter that doesn't seems really needed and have a side effect (different count of returned parameters) that could be surpricing by the parameter function.

@XeCycle
Copy link
Contributor Author

XeCycle commented Dec 30, 2024

Unfortunately there would a problem with the async part: the iterator object is returned immediately, however we cannot tell its version yet. It depends on the first chunk from server, so has to happen asynchronously. I also thought about this; we can rely on StopAsyncIteration on our part, and provide a utility e.g. def store_async_iterator_result(iter) -> IterWithReturnValue, and access it on IterWithReturnValue.result. But accessing it too early would be problematic, e.g. an AttributeError that cannot be checked by static checkers, or a redundant |None part.

@gtsystem
Copy link
Owner

gtsystem commented Jan 1, 2025

Unfortunately there would a problem with the async part: the iterator object is returned immediately, however we cannot tell its version yet. It depends on the first chunk from server, so has to happen asynchronously.

We can do that having a method for resourceVersion instead of an attribute.See #86 for a working implementation.

Usage:

pods = client.list(Pod)
print(await pods.resourceVersion())   # this will trigger the first fetch
async for pod in pods:
     print(pod.metadata.name)
print(await pods.resourceVersion())  # this won't

This implementation is a bit complex to support fetching resourceVersion before starting the for loop. In alternative we could have a simple implementation where resourceVersion() will just raise an exception if it's called too early (and we should also document this behavior).

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