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

Lifecycle management of decorators #609

Open
Dreamsorcerer opened this issue Dec 31, 2022 · 3 comments
Open

Lifecycle management of decorators #609

Dreamsorcerer opened this issue Dec 31, 2022 · 3 comments
Milestone

Comments

@Dreamsorcerer
Copy link
Member

Cache objects should be closed at the end of an application lifecycle, with await Cache.close(), or using async with.

The current design of decorators creates a new cache instance with each decorator and no attempt to close it is made, thus failing to manage this lifecycle management properly.

e.g.

@cached(...)  # New cache is created here, but never closed.
def foo(): ...

We also want to consider using aiojobs for managing some background tasks, but this additionally requires being created within a running loop, something which is unlikely when a decorator is called.


One solution I can think of, is to explicitly manage the caches, and pass them to the decorators. This may also need a .start() method to initiate the cache later. e.g.

cache = Cache(...)

@cached(cache)
def foo(): ...

async def main():
    # Initialise application
    cache.start()
    # Run application
    ...
    # Cleanup application
    await cache.close()

Or more succinctly:

async def main():
    async with cache:
        # Run application
@Dreamsorcerer
Copy link
Member Author

These are causing warnings in our examples as well: #610.

@cancan101
Copy link

The current design of decorators creates a new cache instance with each decorator

Is that true if an alias is used? ie if I use the same alias for multiple decorators will the underlying Cache instance be shared?

@Dreamsorcerer
Copy link
Member Author

The current design of decorators creates a new cache instance with each decorator

Is that true if an alias is used? ie if I use the same alias for multiple decorators will the underlying Cache instance be shared?

Yes, that should be shared, but the issue is really around the lifecycle of the cache, not the sharing. With an alias, the cache is still created on-demand and users are unlikely to think about closing the cache at any point.

I also don't see any real advantage to an alias like that, compared with just passing an explicit reference to the cache object directly.

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

No branches or pull requests

2 participants