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

Migrate to iovalkey #12

Closed
MonsignorEduardo opened this issue Oct 21, 2024 · 7 comments
Closed

Migrate to iovalkey #12

MonsignorEduardo opened this issue Oct 21, 2024 · 7 comments
Assignees
Labels
enhancement New feature or request

Comments

@MonsignorEduardo
Copy link

MonsignorEduardo commented Oct 21, 2024

@MonsignorEduardo MonsignorEduardo changed the title Migrate to official iovalkey Migrate to iovalkey Oct 21, 2024
@yxx4c
Copy link
Owner

yxx4c commented Oct 25, 2024

@MonsignorEduardo ,

Thank you for bringing iovalkey to my attention. I will look into this library and may consider migrating to it once I can rewrite the section that handles auto-caching to avoid the use of async-cache-dedupe. At that point, users won't need to pass a Redis client to the extension, as everything will be managed internally. The choice of any external client won't be a concern unless you're particularly worried about project dependencies.

@yxx4c yxx4c self-assigned this Oct 26, 2024
@yxx4c yxx4c added the enhancement New feature or request label Oct 26, 2024
@yxx4c
Copy link
Owner

yxx4c commented Nov 6, 2024

Hi @MonsignorEduardo,

I have pushed an updated version of the extension with iovalkey to the dev branch, feel free to check it out and share your feedback.

@li2109
Copy link

li2109 commented Nov 9, 2024

Hey @yxx4c, i feel like having a specific redis implementation lock-in is bit a wrong path for such a generic extension. Maybe abstract the operations into an interface so that people can swap the underlying cache impl?

@yxx4c
Copy link
Owner

yxx4c commented Nov 10, 2024

Hii @li2109,

Thank you for sharing your thoughts! I understand the desire for an interface that allows users to provide their own Redis client. However, I have some concerns about this approach.

Introducing an abstraction for the caching operations could lead to potential issues with implementation stability. Any changes to the abstract operations or future modifications might break existing implementations, and I'm not fully aware of the various libraries that support features like pipeline, unlink, and how they handle these functionalities.

Regarding the extension, the primary goal is to provide a caching solution for Prisma query results. Users need a Redis cache that supports auto-caching, custom key caching, and cache invalidation. Initially, the extension required users to pass an ioredis client, but with the switch to iovalkey, I believe this change simplifies the process, as users are no longer required to pass a client just for the extension to use. Although the switch to iovalkey did not change how the internal operations were performed, it does mean that the client is now initialized and used directly within the extension. This gives me the ability to enhance the code as long as the core logic is maintained. If users want to utilize the client inside the extension, they can do so by calling extendedPrisma.redis, which maintains flexibility.

I appreciate your feedback and will keep it in mind for future enhancements. If you have any further suggestions or specific use cases in mind, I’d be happy to discuss them!

@li2109
Copy link

li2109 commented Nov 10, 2024

Hii @li2109,

Thank you for sharing your thoughts! I understand the desire for an interface that allows users to provide their own Redis client. However, I have some concerns about this approach.

Introducing an abstraction for the caching operations could lead to potential issues with implementation stability. Any changes to the abstract operations or future modifications might break existing implementations, and I'm not fully aware of the various libraries that support features like pipeline, unlink, and how they handle these functionalities.

Regarding the extension, the primary goal is to provide a caching solution for Prisma query results. Users need a Redis cache that supports auto-caching, custom key caching, and cache invalidation. Initially, the extension required users to pass an ioredis client, but with the switch to iovalkey, I believe this change simplifies the process, as users are no longer required to pass a client just for the extension to use. Although the switch to iovalkey did not change how the internal operations were performed, it does mean that the client is now initialized and used directly within the extension. This gives me the ability to enhance the code as long as the core logic is maintained. If users want to utilize the client inside the extension, they can do so by calling extendedPrisma.redis, which maintains flexibility.

I appreciate your feedback and will keep it in mind for future enhancements. If you have any further suggestions or specific use cases in mind, I’d be happy to discuss them!

Thank you for your explanation and it does make sense to me. I was thinking about abstraction when i found out that ioredis was not compatible for edge so that it can't be used in cloudflare worker. Not sure about iovalkey though.

Now i feel like abstraction is not important as long as it's edge compatible and user should be able to plug in out of the box with redis url and password.

FYI: i was using @upstash/redis/cloudflare for edge

@yxx4c
Copy link
Owner

yxx4c commented Nov 15, 2024

Hi @li2109,

Thank you for your feedback! I will take it into account as I consider developing an edge-compatible integration or a separate library in the future. In the meantime, I will focus on improving the current library while also exploring better integrations and features that can benefit our community.

@yxx4c
Copy link
Owner

yxx4c commented Nov 15, 2024

Migrated to iovalkey in this commit 97b97c6

@yxx4c yxx4c closed this as completed Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants