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

Is it abandoned? #2

Open
edezhic opened this issue Nov 28, 2024 · 6 comments
Open

Is it abandoned? #2

edezhic opened this issue Nov 28, 2024 · 6 comments

Comments

@edezhic
Copy link

edezhic commented Nov 28, 2024

Currently it only supports a bit outdated version of gluesql (0.15 vs 0.16), are there any plans to update it and maintain in the future?

@kanekoshoyu
Copy link
Owner

Hi, I have not actively maintained this one but let me have a look rn

@kanekoshoyu
Copy link
Owner

seems like its got to do with them adding lifetime in RowIter<'a>, I need some more time to get this fixed, will update this week

@edezhic
Copy link
Author

edezhic commented Dec 12, 2024

I made a temporary solution by copypasting inner sled scan_data fn, but that also required forking gluesql_sled_storage to pub some inner modules. No luck with attempts to wrap the return type into some new stream that holds the lock

@kanekoshoyu
Copy link
Owner

I made a temporary solution by copypasting inner sled scan_data fn, but that also required forking gluesql_sled_storage to pub some inner modules. No luck with attempts to wrap the return type into some new stream that holds the lock

thank you, apologies I did have a look but got distracted by other things on my hand as it was a bit more complicated changes than I expected. Is your fork in public or private? feel free to make pull requests.

@edezhic
Copy link
Author

edezhic commented Dec 13, 2024

haven't pushed yet as it depends on this abomination - gluesql/gluesql@main...edezhic:gluesql:main , and for the impl SharedSledStorage I've added a method scan_inner_data which takes the lock and proceeds with a copypaste of the original SledStorage Store::scan_data code. It resolves the issue but definitely isn't a good way forward.

Have you considered merging SharedSledStorage into the original one or at least pushing it to the gluesql repo as an additional storage option? I think SharedSledStorage doesn't add significant overhead to the SledStorage and concurrent-safe version seems like a good default. Considering that Sled can handle concurrency under the hood and gluesql is built with concurrency in mind I think that the default sled gluesql storage should be safe to use in concurrent contexts. Also, by merging these things it would be easier to maintain compatibility. Wdyt?

@edezhic
Copy link
Author

edezhic commented Dec 15, 2024

I've pushed this temporary solution here - https://github.com/edezhic/prest/blob/main/host/shared_sled_storage.rs

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