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

support execute_async #366

Merged
merged 8 commits into from
Oct 2, 2021
Merged

support execute_async #366

merged 8 commits into from
Oct 2, 2021

Conversation

24seconds
Copy link
Collaborator

@24seconds 24seconds commented Oct 2, 2021

Hello, while I'm using gluesql as a database (MemoryStorage),

Motivation

I found that glue blocks current thread until the execution is finished. For me, I need asynchronous execute function, not blocking one. So I wrote some code to support it.

Question

I saw so many test cases are written in this repo. So I also think I should write a test but I don't know where to start. Could you give me some guide? Thank you!

@24seconds 24seconds requested a review from panarch October 2, 2021 08:16
@24seconds 24seconds self-assigned this Oct 2, 2021
@24seconds 24seconds added the enhancement New feature or request label Oct 2, 2021
@codecov-commenter
Copy link

codecov-commenter commented Oct 2, 2021

Codecov Report

Merging #366 (d7fa1e5) into main (a58369a) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #366      +/-   ##
==========================================
+ Coverage   91.22%   91.23%   +0.01%     
==========================================
  Files         133      133              
  Lines        8259     8274      +15     
==========================================
+ Hits         7534     7549      +15     
  Misses        725      725              
Impacted Files Coverage Δ
src/glue.rs 100.00% <100.00%> (ø)
tests/sled_transaction.rs 99.51% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a58369a...d7fa1e5. Read the comment docs.

@24seconds
Copy link
Collaborator Author

After the guide, I wrote the test in examples/api_usage.rs.
While doing that, I found 2 things and modified them with help from taehoon.

  • mutable_api has some bug. It doens't run rest sqls, insert, drop etc. So I make them run in for loop
  • In the glue.rs, plan function uses block_on internal. After talk with him, I changed this function as asynchronous

@24seconds
Copy link
Collaborator Author

Added dedicated test for execute_async. Checked using tarpualin and it's 100% for glue.rs file.

Copy link
Member

@panarch panarch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome 👍
Thanks!!

Copy link
Member

@ever0de ever0de left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@panarch panarch merged commit 00c3c08 into gluesql:main Oct 2, 2021
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

Successfully merging this pull request may close these issues.

None yet

4 participants