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: filter.createSubscription accept ShardParams #1967

Closed
vpavlin opened this issue Apr 19, 2024 · 3 comments
Closed

feat: filter.createSubscription accept ShardParams #1967

vpavlin opened this issue Apr 19, 2024 · 3 comments

Comments

@vpavlin
Copy link
Member

vpavlin commented Apr 19, 2024

support request

Problem

It seems like filter.createSubscription accepts either pubsubTopic or SingleShardInfo - which in turn consists of ClusterId and shard

Would it make more sense to accept ShardingParams to make it more flexible?

Requiring SingleShardInfo forces me to get the shard from contentTopic before calling createSubscription, which seems unnecessary to be done by a user/dev.

But I might be missing the reasoning behind this choice:)

Proposed Solutions

Notes

@danisharora099
Copy link
Collaborator

Looks right to me. Good point. Thanks for flagging this @vpavlin

Looks like the filter API wasn't updated after we introduced autosharding. Moving to priority.

@adklempner are you happy to address this?

@danisharora099 danisharora099 moved this to Priority in Waku Apr 22, 2024
@weboko
Copy link
Collaborator

weboko commented Apr 22, 2024

I agree with @vpavlin and I think better way would be to simplify as much as possible but also allow to make fine tuning.

For that I see we can do:

  • make it accept contentTopic and map it to a shard;
  • provide API for advanced users to set particular shard or other params;

Since I am touching this code in #1958 I might be able to address it there.

@weboko weboko added the enhancement New feature or request label Apr 24, 2024
@weboko weboko changed the title Why does filter.createSubscription accept SingleShardInfo feat: filter.createSubscription accept ShardParams Apr 24, 2024
@weboko weboko moved this from Priority to In Progress in Waku Apr 24, 2024
@weboko weboko moved this from In Progress to Code Review / QA in Waku Apr 29, 2024
@weboko weboko moved this from Code Review / QA to Done in Waku Apr 29, 2024
@danisharora099
Copy link
Collaborator

@weboko issue seems to be in the "done" column but still open -- I guess we can close it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

4 participants