R4R: Support query txs' TotalCount in GET /txs #4214

yangyanqing · 2019-04-27T09:34:03Z

Targeted PR against correct branch (see CONTRIBUTING.md)

close: #3942
replace: #3995

Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
Wrote tests
Updated relevant documentation (docs/)
Added a relevant changelog entry: sdkch add [section] [stanza] [message]
rereviewed Files changed in the github PR explorer

For Admin Use:

Added appropriate labels to PR (ex. wip, ready-for-review, docs)
Reviewers Assigned
Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

fedekunze

Minor comments only, otherwise LGTM

types/result.go

codecov · 2019-04-27T10:25:33Z

Codecov Report

Merging #4214 into master will decrease coverage by 0.02%.
The diff coverage is 38.46%.

@@            Coverage Diff             @@
##           master    #4214      +/-   ##
==========================================
- Coverage   60.16%   60.13%   -0.03%     
==========================================
  Files         212      212              
  Lines       15188    15197       +9     
==========================================
+ Hits         9138     9139       +1     
- Misses       5421     5429       +8     
  Partials      629      629

codecov · 2019-04-27T10:25:34Z

Codecov Report

Merging #4214 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #4214      +/-   ##
==========================================
- Coverage   59.14%   59.12%   -0.02%     
==========================================
  Files         217      217              
  Lines       14587    14595       +8     
==========================================
+ Hits         8627     8629       +2     
- Misses       5322     5328       +6     
  Partials      638      638

fedekunze

I forgot to mention that we also need to update the documentation (swagger and the gaiacli docs)

yangyanqing · 2019-04-29T02:42:58Z

I forgot to mention that we also need to update the documentation (swagger and the gaiacli docs)

Sorry, I forgot it and had updated swagger.
But gaiacli doc had nothing with detail of rest endpoint.

@fedekunze

fedekunze

ACK, Thanks !

alexanderbez

One minor nit, but otherwise LGTM.

client/lcd/swagger-ui/swagger.yaml

.pending/breaking/gaiarest/3942-Support-query-t

client/lcd/swagger-ui/swagger.yaml

jackzampolin · 2019-04-30T18:05:49Z

types/result.go

@@ -249,6 +250,27 @@ func (r TxResponse) Empty() bool {
 	return r.TxHash == "" && r.Logs == nil
 }

+// SearchTxsResult defines a structure for querying txs pageable
+type SearchTxsResult struct {
+	TotalCount int          `json:"total_count"` // Count of all txs


Does this need to returned with every transaction search query? Should this be it's own query?

I think any query which does not return all items one time should be pageable. This is friendly to the user. @jackzampolin

types/result.go

fedekunze · 2019-05-03T13:48:17Z

it seems the tests are failing now

yangyanqing · 2019-05-03T14:03:46Z

Branch master's CI was fail, so this PR was fail after rebasing master just now. @fedekunze

alessio · 2019-05-03T14:17:22Z

This should do it: #4264

alessio · 2019-05-03T16:08:53Z

@yangyanqing please rebase again

Co-Authored-By: yangyanqing <[email protected]>

yangyanqing · 2019-05-03T23:29:55Z

It's ok on my local workspace, but still fail on circle. I have remove go.sum and build/ before run test.

➜  cosmos-sdk git:(frank/3942-support-TotalCount-in-GET-txs-master) ✗ make test_cli       
go build -mod=readonly -tags "netgo ledger" -ldflags '-X github.com/cosmos/cosmos-sdk/version.Name=gaia -X github.com/cosmos/cosmos-sdk/version.Version=0.34.0-hashgard-84-g16dd2f533 -X github.com/cosmos/cosmos-sdk/version.Commit=16dd2f533015a039b703ca44b7146a693e4baec4 -X "github.com/cosmos/cosmos-sdk/version.BuildTags=netgo ledger" -X github.com/cosmos/cosmos-sdk/version.GoSumHash=c4e937a993d3a6f251c77a73a959408b907c393098cc05e3ae5abb49d0a50aa7' -o build/gaiad ./cmd/gaia/cmd/gaiad
go build -mod=readonly -tags "netgo ledger" -ldflags '-X github.com/cosmos/cosmos-sdk/version.Name=gaia -X github.com/cosmos/cosmos-sdk/version.Version=0.34.0-hashgard-84-g16dd2f533 -X github.com/cosmos/cosmos-sdk/version.Commit=16dd2f533015a039b703ca44b7146a693e4baec4 -X "github.com/cosmos/cosmos-sdk/version.BuildTags=netgo ledger" -X github.com/cosmos/cosmos-sdk/version.GoSumHash=c4e937a993d3a6f251c77a73a959408b907c393098cc05e3ae5abb49d0a50aa7' -o build/gaiacli ./cmd/gaia/cmd/gaiacli
go build -mod=readonly -tags "netgo ledger" -ldflags '-X github.com/cosmos/cosmos-sdk/version.Name=gaia -X github.com/cosmos/cosmos-sdk/version.Version=0.34.0-hashgard-84-g16dd2f533 -X github.com/cosmos/cosmos-sdk/version.Commit=16dd2f533015a039b703ca44b7146a693e4baec4 -X "github.com/cosmos/cosmos-sdk/version.BuildTags=netgo ledger" -X github.com/cosmos/cosmos-sdk/version.GoSumHash=c4e937a993d3a6f251c77a73a959408b907c393098cc05e3ae5abb49d0a50aa7' -o build/gaiareplay ./cmd/gaia/cmd/gaiareplay

ok      github.com/cosmos/cosmos-sdk/cmd/gaia/cli_test  181.706s
➜  cosmos-sdk git:(frank/3942-support-TotalCount-in-GET-txs-master) ✗ make test_sim_gaia_nondeterminism
......
Selected randomly generated distribution parameters:
        {FeePool:{CommunityPool:} CommunityTax:0.200000000000000000 BaseProposerReward:0.110000000000000000 BonusProposerReward:0.180000000000000000 WithdrawAddrEnabled:false DelegatorWithdrawInfos:[] PreviousProposer: OutstandingRewards:[] ValidatorAccumulatedCommissions:[] ValidatorHistoricalRewards:[] ValidatorCurrentRewards:[] DelegatorStartingInfos:[] ValidatorSlashEvents:[]}
Simulating... block 50/50, operation 0/0. . . 
Simulation complete; Final height (blocks): 51, final time (seconds): 2410-01-12 09:02:40 +0800 CST, operations ran: 2867
Event statistics: 
                                         /no-operation/failure => 184
                                       auth/deduct_fee/failure => 1
                                            auth/deduct_fee/ok => 17
                                        bank/multisend/failure => 4
                                             bank/multisend/ok => 26
                                             bank/send/failure => 20
                                                  bank/send/ok => 335
                                           beginblock/evidence => 9
                                     beginblock/signing/missed => 3767
                                     beginblock/signing/signed => 3526
                            distr/set_withdraw_address/failure => 163
                       distr/withdraw_delegator_reward/failure => 146
                            distr/withdraw_delegator_reward/ok => 4
                   distr/withdraw_validator_commission/failure => 151
                               endblock/validatorupdates/added => 49
                              endblock/validatorupdates/kicked => 49
                             endblock/validatorupdates/updated => 342
                                           gov/deposit/failure => 306
                                                gov/deposit/ok => 16
                                   gov/submit_proposal/failure => 3
                                        gov/submit_proposal/ok => 33
                                       slashing/unjail/failure => 335
                              staking/begin_redelegate/failure => 250
                                   staking/begin_redelegate/ok => 2
                              staking/create_validator/failure => 272
                                      staking/delegate/failure => 159
                                           staking/delegate/ok => 419
                                staking/edit_validator/failure => 1
                                     staking/edit_validator/ok => 20
--- PASS: TestAppStateDeterminism (164.59s)
PASS
ok      github.com/cosmos/cosmos-sdk/cmd/gaia/app       164.648s

@fedekunze @alessio

yangyanqing · 2019-05-04T01:15:47Z

It's clear. @fedekunze @alessio

yangyanqing requested review from alessio, alexanderbez, jackzampolin and rigelrozanski as code owners April 27, 2019 09:34

fedekunze suggested changes Apr 27, 2019

View reviewed changes

types/result.go Outdated Show resolved Hide resolved

types/result.go Show resolved Hide resolved

types/result.go Outdated Show resolved Hide resolved

alexanderbez added ready-for-review labels Apr 27, 2019

fedekunze reviewed Apr 28, 2019

View reviewed changes

fedekunze approved these changes Apr 29, 2019

View reviewed changes

alexanderbez suggested changes Apr 29, 2019

View reviewed changes

client/lcd/swagger-ui/swagger.yaml Outdated Show resolved Hide resolved

.pending/breaking/gaiarest/3942-Support-query-t Outdated Show resolved Hide resolved

alexanderbez suggested changes Apr 29, 2019

View reviewed changes

client/lcd/swagger-ui/swagger.yaml Outdated Show resolved Hide resolved

alexanderbez approved these changes Apr 30, 2019

View reviewed changes

jackzampolin reviewed Apr 30, 2019

View reviewed changes

alessio reviewed May 2, 2019

View reviewed changes

types/result.go Outdated Show resolved Hide resolved

Frank Yang and others added 9 commits May 4, 2019 06:47

Support query txs' TotalCount in GET /txs #3942

dbf6bec

Support query txs' TotalCount in GET /txs #3942

d4f1ffe

Support query txs' TotalCount in GET /txs #3942

cb7f53f

Rebase to resolve conflict.

5102e80

Add comments for each property's definition.

b86b439

Fix swagger.yaml

b188cfd

Update client/lcd/swagger-ui/swagger.yaml

7e3762c

Co-Authored-By: yangyanqing <[email protected]>

Update .pending/breaking/gaiarest/3942-Support-query-t

b183897

Co-Authored-By: yangyanqing <[email protected]>

Update client/lcd/swagger-ui/swagger.yaml

fc705c0

Co-Authored-By: yangyanqing <[email protected]>

Frank Yang added 2 commits May 4, 2019 06:47

Rename PageCount to PageTotal

874be42

Fix text fail.

16dd2f5

alessio merged commit 1cfc868 into cosmos:master May 4, 2019

yangyanqing deleted the frank/3942-support-TotalCount-in-GET-txs-master branch May 4, 2019 13:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

R4R: Support query txs' TotalCount in GET /txs #4214

R4R: Support query txs' TotalCount in GET /txs #4214

yangyanqing commented Apr 27, 2019 •

edited by alexanderbez

Loading

fedekunze left a comment

codecov bot commented Apr 27, 2019

codecov bot commented Apr 27, 2019 •

edited

Loading

fedekunze left a comment

yangyanqing commented Apr 29, 2019

fedekunze left a comment

alexanderbez left a comment

jackzampolin Apr 30, 2019

yangyanqing May 1, 2019

fedekunze commented May 3, 2019

yangyanqing commented May 3, 2019 •

edited

Loading

alessio commented May 3, 2019

alessio commented May 3, 2019

yangyanqing commented May 3, 2019 •

edited

Loading

yangyanqing commented May 4, 2019

R4R: Support query txs' TotalCount in GET /txs #4214

R4R: Support query txs' TotalCount in GET /txs #4214

Conversation

yangyanqing commented Apr 27, 2019 • edited by alexanderbez Loading

fedekunze left a comment

Choose a reason for hiding this comment

codecov bot commented Apr 27, 2019

Codecov Report

codecov bot commented Apr 27, 2019 • edited Loading

Codecov Report

fedekunze left a comment

Choose a reason for hiding this comment

yangyanqing commented Apr 29, 2019

fedekunze left a comment

Choose a reason for hiding this comment

alexanderbez left a comment

Choose a reason for hiding this comment

jackzampolin Apr 30, 2019

Choose a reason for hiding this comment

yangyanqing May 1, 2019

Choose a reason for hiding this comment

fedekunze commented May 3, 2019

yangyanqing commented May 3, 2019 • edited Loading

alessio commented May 3, 2019

alessio commented May 3, 2019

yangyanqing commented May 3, 2019 • edited Loading

yangyanqing commented May 4, 2019

yangyanqing commented Apr 27, 2019 •

edited by alexanderbez

Loading

codecov bot commented Apr 27, 2019 •

edited

Loading

yangyanqing commented May 3, 2019 •

edited

Loading

yangyanqing commented May 3, 2019 •

edited

Loading