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 SQL Storage #260

Open
wants to merge 39 commits into
base: master
Choose a base branch
from
Open

Conversation

Avihais12344
Copy link

Hello,

I was wondering, why there's no General SQL Storage. So I have added it.

I have used SQLalchemy for the task, and for some weird reason, it doesn't work with trio (async backend), but I think we can still add it.

Thanks ahead for the help!

@Avihais12344
Copy link
Author

Also I need to add the remove functionality, i would add it. Please don't close this PR.

@Avihais12344
Copy link
Author

Now added the remove!

@karpetrosyan
Copy link
Owner

Hi! Thanks for this PR. It would be very useful to have a general SQL implementation as a storage option, but I think it will make our storage solutions more complicated to maintain. So, I suggest continuing with the existing storage options and introducing the new one as a separate library. I’d be happy to add related libraries to hishel's documentation.

@Avihais12344
Copy link
Author

Why do you think it would be more complicated to maintain? You already have many options, we can replace the sqlite one with this, and have more options like postgress, mysql and more. I think the maintenance is worth the cost. For some people, get SQL-ish DB is way easier then getting S3 or redis.

@Avihais12344
Copy link
Author

Finally it passes!

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