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

refactor: use protobuffer for API storenode queries #1248

Merged
merged 2 commits into from
Oct 24, 2024

Conversation

richard-ramos
Copy link
Member

To allow more flexibility at the moment of doing queries, instead of having custom functions that receive N different combination of parameters to do a store query, we use instead a single function which receives instead a StoreQueryRequest protobuffer

The json representation of the StoreQueryRequest matches what is used in libwaku, and since protobuffers in nwaku and go-waku are the same (and are supposed to be backward compatible), we can safely use them. It has the side effect of making the API simpler too

cc: @gabrielmer, @Ivansete-status

@status-im-auto
Copy link

status-im-auto commented Oct 22, 2024

Jenkins Builds

Click to see older builds (1)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 5dc634b #1 2024-10-22 13:43:36 ~3 min unknown 📄log
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 212f697 #2 2024-10-24 18:17:46 ~2 min unknown 📄log
✔️ b6fac0f #3 2024-10-24 18:43:13 ~2 min unknown 📄log
✔️ b6fac0f #4 2024-10-24 18:43:33 ~13 sec unknown 📄log

@@ -0,0 +1,12 @@
package common
Copy link
Contributor

Choose a reason for hiding this comment

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

can you make the file snake case? storenode_requestor.go?

Copy link
Member Author

Choose a reason for hiding this comment

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

yup

Copy link
Collaborator

@chaitanyaprem chaitanyaprem left a comment

Choose a reason for hiding this comment

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

LGTM


var params Parameters
params.selectedPeer = peerID
if params.selectedPeer == "" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am guessing peer-selection would be added later.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. I want to get the store queries to work first as they do now, with nwaku

@richard-ramos richard-ramos merged commit fdb3c3d into master Oct 24, 2024
12 checks passed
@richard-ramos richard-ramos deleted the feat/gen-query branch October 24, 2024 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants