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

Make profiles query user parameter optional #534

Closed
RiccardoM opened this issue Jul 5, 2021 · 5 comments · Fixed by #539
Closed

Make profiles query user parameter optional #534

RiccardoM opened this issue Jul 5, 2021 · 5 comments · Fixed by #539
Assignees
Labels
kind/enhancement Enhance an already existing feature; no "New feature" to add x/profiles Module that allows to create and manage decentralized social profiles
Milestone

Comments

@RiccardoM
Copy link
Contributor

Context

Currently when querying the x/profiles module data, almost all queries require to specify a single user to which to query the data for. Examples:

  • query blocks [user]
  • query relationships [user]
  • query incoming-dtag-transfer-requests [user]
  • ...

While this is surely handy, I think we should allow developers to also query for all such data without specifying a single user.

Proposal

My proposal is to make the [user] parameter optional on all queries, allowing to get all the stored data with pagination:

  • query blocks [[user]]
  • query relationships [[user]]
  • query dtag-transfer-requests
    Note. This command should be a different one from the incoming-dtag-transfer-requests to avoid any confusion
  • ...

Please let me know what you guys think @bragaz @dadamu

@RiccardoM RiccardoM added kind/enhancement Enhance an already existing feature; no "New feature" to add x/profiles Module that allows to create and manage decentralized social profiles labels Jul 5, 2021
@RiccardoM RiccardoM added this to the v0.18.0 milestone Jul 5, 2021
@leobragaz
Copy link
Contributor

This sounds good to me 👍 , plus it can be done easily now with all the paginated requests

@dadamu
Copy link
Contributor

dadamu commented Jul 5, 2021

It sounds good to me, too. 👍
However, to query all the stored data with offset in pagination config would search through all the records (doc). That may be a problem if there are the huge number of data in store.
Should we also limit the Offset and Limit in PageRequest?

@dadamu dadamu self-assigned this Jul 5, 2021
@leobragaz
Copy link
Contributor

@dadamu maybe it's a good idea

@RiccardoM
Copy link
Contributor Author

Should we also limit the Offset and Limit in PageRequest?

I think we can avoid doing this. Developers will use the NextKey field anyway instead of Offset and Limit. I think we should keep them as they are without any worry. The worst thing it can happen is they're going to have a very slow response and possibly block their node. We won't have any public RPC anyway.

@kwunyeung
Copy link
Contributor

From my experience, most issue due to large return result is from gRPC gateway. At least CLI can handle it but just like what @RiccardoM mentioned the user just has to wait.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Enhance an already existing feature; no "New feature" to add x/profiles Module that allows to create and manage decentralized social profiles
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants