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

feat(store): add waku store client module #1229

Merged
merged 1 commit into from
Oct 20, 2022
Merged

feat(store): add waku store client module #1229

merged 1 commit into from
Oct 20, 2022

Conversation

LNSD
Copy link
Contributor

@LNSD LNSD commented Oct 5, 2022

The waku store protocol is a request-response/client-server protocol. These changes decouple the client logic from the request-handling logic. So now it should be possible to do waku store history queries without mounting the store protocol handling/server part.

  • Move waku store client logic to waku_store/client module
  • Deprecate waku store protocol client procedures
  • Use waku store client in waku store unit tests
  • Extend coverage of waku store client code (see tests/v2/test_waku_store_client.nim)

Remaining work:

  • Use the waku store client in wakunode2/waku_node.
  • Remove waku store protocol deprecated methods.

@LNSD LNSD self-assigned this Oct 5, 2022
@status-im-auto
Copy link
Collaborator

status-im-auto commented Oct 5, 2022

Jenkins Builds

Click to see older builds (15)
Commit #️⃣ Finished (UTC) Duration Platform Result
d232fd7 #1 2022-10-05 11:12:10 ~7 min linux 📄log
d232fd7 #1 2022-10-05 11:13:05 ~8 min macos 📄log
8a76365 #2 2022-10-05 18:56:18 ~5 min linux 📄log
8a76365 #2 2022-10-05 18:59:33 ~9 min macos 📄log
7a89b8f #3 2022-10-12 03:47:12 ~5 min linux 📄log
7a89b8f #3 2022-10-12 03:58:27 ~16 min macos 📄log
b3232b2 #4 2022-10-13 22:54:32 ~7 min macos 📄log
3d1ba3c #4 2022-10-17 23:56:01 ~5 min linux 📄log
3d1ba3c #5 2022-10-17 23:59:23 ~8 min macos 📄log
cb5e08f #5 2022-10-19 18:12:05 ~10 min linux 📄log
cb5e08f #6 2022-10-19 18:12:15 ~10 min macos 📄log
cb5e08f #6 2022-10-19 19:26:19 ~1 min linux 📄log
5e948f8 #8 2022-10-19 19:52:57 ~14 min linux 📄log
✔️ 5e948f8 #8 2022-10-19 19:57:51 ~19 min macos 📦bin
✔️ 5e948f8 #9 2022-10-19 21:09:10 ~14 min linux 📦bin
Commit #️⃣ Finished (UTC) Duration Platform Result
aa0937c #10 2022-10-20 13:14:40 ~18 min linux 📄log
aa0937c #9 2022-10-20 13:17:48 ~21 min macos 📄log
1c96807 #11 2022-10-20 13:14:57 ~16 min linux 📄log
✔️ 1c96807 #10 2022-10-20 13:15:57 ~17 min macos 📦bin
✔️ 1c96807 #12 2022-10-20 13:54:58 ~16 min linux 📦bin

@LNSD LNSD marked this pull request as draft October 5, 2022 11:54
@LNSD LNSD force-pushed the feat-store-client branch 3 times, most recently from b3232b2 to 3d1ba3c Compare October 17, 2022 23:50
@LNSD LNSD force-pushed the feat-store-client branch 3 times, most recently from 5e948f8 to aa0937c Compare October 20, 2022 12:56
@LNSD LNSD marked this pull request as ready for review October 20, 2022 13:44
Copy link
Contributor

@alrevuelta alrevuelta left a comment

Choose a reason for hiding this comment

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

looks great!

tests/v2/test_waku_store_client.nim Show resolved Hide resolved
## Returns all the fetched messages, if error occurs, returns an error string

# Make a copy of the query
var req = query
Copy link
Contributor

Choose a reason for hiding this comment

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

out of curiosity, why do you need a copy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To update the cursor field, and keep the rest of the request intact. This part of the implementation has not been changed compared to the original protocol.nim code.

Copy link
Contributor

@jm-clius jm-clius left a comment

Choose a reason for hiding this comment

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

LGTM! Can't wait to not mount store protocol on store clients which ultimately allows us to greatly simplify config.

@LNSD LNSD requested a review from alrevuelta October 20, 2022 16:08
@LNSD LNSD merged commit e5c3aa5 into master Oct 20, 2022
@LNSD LNSD deleted the feat-store-client branch October 20, 2022 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants