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

Update gluesql #142

Merged
merged 1 commit into from
Dec 25, 2023
Merged

Update gluesql #142

merged 1 commit into from
Dec 25, 2023

Conversation

serprex
Copy link
Contributor

@serprex serprex commented Dec 24, 2023

gluesql made all their methods async,
but even replacing Mutex with tokio's async Mutex doesn't solve borrowing issues, seems gluesql isn't tokio compatible, so execute with futures::executor::block_on

Fixes #127

gluesql made all their methods async,
but even replacing Mutex with tokio's async Mutex doesn't solve borrowing issues,
seems gluesql isn't tokio compatible, so execute with futures::executor::block_on
@sunng87
Copy link
Owner

sunng87 commented Dec 25, 2023

Thank you! I was trying to use async gluesql in our query APIs and failed. Since it's all memory based operation, it's ok to use blocked operation here.

@sunng87 sunng87 merged commit 9e5caaf into sunng87:master Dec 25, 2023
6 checks passed
@ever0de
Copy link

ever0de commented Dec 26, 2023

hello @sunng87 :)

I commented on the follow-up story.
gluesql/gluesql#1247 (comment)

The existing sync API was also implemented using block_on, so I think it's doing the same thing.
image

example of a cli workspace
image

Now that I checked the above, the problem is caused by gluesql not supporting Send.
The reason we only supported the async api was to make the block_on api optional when the sync api was needed at the user level.

Thanks for the great example!

@sunng87
Copy link
Owner

sunng87 commented Dec 27, 2023

Hey @ever0de ! Thank you for the clarification. I think you have a plan to make gluesql Send, I will update the example when it happens

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.

3 participants