community[minor]: Add async methods to AstraDBCache #17415
Conversation
dosubot
bot
added
the
size:XL
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
dosubot
bot
added
Ɑ: memory
| ) | ||
|
|
||
|
|
||
| class AstraDBCollectionEnvironment(AstraDBEnvironment): |
There was a problem hiding this comment.
- Would you mind adding doc-strings explaining what this is?
- Could we prefix this with a
_to mark is as an internal object? I don't think that we expect most LangChain users to know about this object?
There was a problem hiding this comment.
ah it was public because it's used in other files (obvious, but I missed that lol). I don't think we have a great convention right now with respect to public/private. I still prefer erring on the side of private and prioritize what users see as public
| if async_setup: | ||
| async_astra_db = self.async_astra_db | ||
|
|
||
| async def _setup_db() -> None: |
There was a problem hiding this comment.
The code block seems to force a network request during initialization.
I left a comment about this above. We don't necessarily need to do anything at this stage, but could be good to allow a developer to opt-out from the init somehow for more advanced use cases
There was a problem hiding this comment.
FYI, the Cassandra vector store is about to receive a skip_provisioning: bool = False flag for exactly that purpose. There might be a same-named parameter here.
There was a problem hiding this comment.
added SetupMode param (SYNC, ASYNC, OFF)
| """ | ||
| astra_env = AstraDBEnvironment( | ||
| self.astra_env = AstraDBCollectionEnvironment( |
There was a problem hiding this comment.
FYI: #17242 -- will make it possible to set caches at request time, so we'll be more likely to see repeated instantiation of objects.
AstraDBCollectionEnvironment makes a network request on every init. OK for now, but could become expensive if user re-instantiates the cache repeatedly (which I think they should be allowed to do)
There was a problem hiding this comment.
added SetupMode param (SYNC, ASYNC, OFF)
| _unset = ["unset"] | ||
|
|
||
|
|
||
| class CachedAwaitable: |
There was a problem hiding this comment.
If these are internal could we prefix them with _ and add a quick doc-string explaining how they are used?
cbornet
force-pushed
the
astra-async-cache
branch
from
1b2b7f1 to
6f638fb
Compare
| async_astra_db_client: Optional[AsyncAstraDB] = None, | ||
| namespace: Optional[str] = None, | ||
| setup_mode: SetupMode = SetupMode.SYNC, | ||
| pre_delete_collection: bool = False, |
There was a problem hiding this comment.
do only i find pre_delete_collection scary? :)
dosubot
bot
added
the
lgtm
eyurtsev
changed the title
eyurtsev
changed the title
No description provided.