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

[schema] #2108: Add pagination #2107

Merged
merged 3 commits into from Apr 20, 2022

Conversation

appetrosyan
Copy link
Contributor

@appetrosyan appetrosyan commented Apr 15, 2022

Signed-off-by: BAStos525 66615487+BAStos525@users.noreply.github.com

Description of the Change

Add pagination support for SDKs.

Issue

Closes #2108

Benefits

Pagination added to queries, so that SDKs have access to the Pagination struct.

@github-actions github-actions bot added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Apr 15, 2022
Arjentix
Arjentix previously approved these changes Apr 15, 2022
@appetrosyan appetrosyan changed the title [ci] load-rs:release workflow triggers [feature] #0000: Add pagination Apr 15, 2022
@appetrosyan appetrosyan changed the title [feature] #0000: Add pagination [feature] #2108: Add pagination Apr 15, 2022
@appetrosyan appetrosyan force-pushed the i2-pagination branch 2 times, most recently from 95f6577 to b8438b1 Compare April 16, 2022 06:16
@appetrosyan appetrosyan marked this pull request as draft April 19, 2022 07:33
@appetrosyan appetrosyan marked this pull request as ready for review April 19, 2022 08:51
@s8sato s8sato added the api-changes Changes in the API for client libraries label Apr 20, 2022
Comment on lines -112 to +114
) -> Result<Scale<VersionedQueryResult>, warp::http::Response<warp::hyper::Body>> {
) -> Result<Scale<VersionedPaginatedQueryResult>, warp::http::Response<warp::hyper::Body>> {
Copy link
Contributor

Choose a reason for hiding this comment

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

The strange thing is that Client still expects a VersionedQueryResult in request_with_pagination, but seems to successfully decode a query result that is actually a VersionedPaginatedQueryResult

Copy link
Contributor

Choose a reason for hiding this comment

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

BTW maybe this PR should be categorized as [schema]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not PR, but commit. If you try to add schema to the PR title, it will not pass CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The strange thing is that Client still expects a VersionedQueryResult in request_with_pagination, but seems to successfully decode a query result that is actually a VersionedPaginatedQueryResult

I'm guessing that it tries to decode the subset that is a VersionedQueryResult, but it should be updated. Good catch.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not PR, but commit. If you try to add schema to the PR title, it will not pass CI.

Timely, #2115 shows an exception

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, later I can resolve the appearance discrepancy between VersionedPaginatedQueryResult and VersionedQueryResult

BAStos525 and others added 2 commits April 20, 2022 12:24
Signed-off-by: BAStos525 <66615487+BAStos525@users.noreply.github.com>
Signed-off-by: Aleksandr Petrosyan <a-p-petrosyan@yandex.ru>
@appetrosyan appetrosyan changed the title [feature] #2108: Add pagination [schema] #2108: Add pagination Apr 20, 2022
@appetrosyan appetrosyan requested a review from s8sato April 20, 2022 08:26
@appetrosyan appetrosyan changed the title [schema] #2108: Add pagination [schema]: Add pagination Apr 20, 2022
@appetrosyan appetrosyan changed the title [schema]: Add pagination [feature] #2108: Add pagination Apr 20, 2022
mversic
mversic previously approved these changes Apr 20, 2022
Signed-off-by: Aleksandr Petrosyan <a-p-petrosyan@yandex.ru>
@appetrosyan appetrosyan changed the title [feature] #2108: Add pagination [schema] #2108: Add pagination Apr 20, 2022
@appetrosyan appetrosyan merged commit ec419e1 into hyperledger:iroha2-dev Apr 20, 2022
@appetrosyan appetrosyan deleted the i2-pagination branch April 20, 2022 09:29
mversic pushed a commit to mversic/iroha that referenced this pull request May 2, 2022
appetrosyan added a commit to appetrosyan/iroha that referenced this pull request May 4, 2022
Signed-off-by: Aleksandr Petrosyan <a-p-petrosyan@yandex.ru>
appetrosyan added a commit to appetrosyan/iroha that referenced this pull request May 12, 2022
appetrosyan added a commit to appetrosyan/iroha that referenced this pull request May 12, 2022
appetrosyan added a commit to appetrosyan/iroha that referenced this pull request May 12, 2022
Signed-off-by: Aleksandr Petrosyan <a-p-petrosyan@yandex.ru>
appetrosyan added a commit to appetrosyan/iroha that referenced this pull request May 12, 2022
Signed-off-by: Aleksandr Petrosyan <a-p-petrosyan@yandex.ru>
appetrosyan added a commit to appetrosyan/iroha that referenced this pull request May 12, 2022
Signed-off-by: Aleksandr Petrosyan <a-p-petrosyan@yandex.ru>
appetrosyan added a commit to appetrosyan/iroha that referenced this pull request May 12, 2022
Signed-off-by: Aleksandr Petrosyan <a-p-petrosyan@yandex.ru>
mversic pushed a commit to mversic/iroha that referenced this pull request May 13, 2022
mversic pushed a commit to mversic/iroha that referenced this pull request May 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-changes Changes in the API for client libraries iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants