Add Trades endpoint in Go SDK #408

0xMichaelRan · 2018-04-17T03:30:53Z

As titled, add a new function LoadTrades()

Testing:

	trades, err = LocalhostCoreClient.LoadTrades(
		horizon.Asset{Type: "native"},
		horizon.Asset{"credit_alphanum4", "SLT", "GCKA6K5PCQ6PNF5RQBF7PQDJWRHO6UOGFMRLK3DYHDOI244V47XKQ4GP"},
		0,
		300000,
		horizon.Limit(3),
		horizon.Order(horizon.OrderAsc),
	)

	fmt.Println("Start printing trades: ")
	for i, rr := range trades.Embedded.Records {
		.... .... 
		fmt.Println(rr)
	}

Result

Start printing trades: 
0
{{ false} {http://localhost:8000/accounts/GBY6A3UQ4RSGWMFDXSIYEDUKJUCXQNDCYPBVLILUZXMJRPGI4PNYA5D7 false} {http://localhost:8000/accounts/GANTILTFDYGNWWJ67PRHTOFYF4ALXBIZ2IVGQHABNN5LBD7MXM4FCH43 false} {http://localhost:8000/operations/73662146575085569 false}}
{{{ false} { false} { false} { false}} 73662146575085569-0 73662146575085569-0 2018-04-03 02:20:33 +0000 UTC 5151620 GBY6A3UQ4RSGWMFDXSIYEDUKJUCXQNDCYPBVLILUZXMJRPGI4PNYA5D7 0.4999200 native   GANTILTFDYGNWWJ67PRHTOFYF4ALXBIZ2IVGQHABNN5LBD7MXM4FCH43 0.1055800 credit_alphanum4 SLT GCKA6K5PCQ6PNF5RQBF7PQDJWRHO6UOGFMRLK3DYHDOI244V47XKQ4GP true 0xc420156398}
1
{{ false} {http://localhost:8000/accounts/GCSBMZV2BYD35GZZVM327APFZEOCHVXWT35ZKHKBSQIEV2H3ATNYWSQA false} {http://localhost:8000/accounts/GAS6QKMRGS76XGKNNQ3MGWUNVQZCBLQAS2KPOITSGD6DPWQZRUQEY5BF false} {http://localhost:8000/operations/73663447950188545 false}}
{{{ false} { false} { false} { false}} 73663447950188545-0 73663447950188545-0 2018-04-03 02:45:45 +0000 UTC 5146821 GCSBMZV2BYD35GZZVM327APFZEOCHVXWT35ZKHKBSQIEV2H3ATNYWSQA 200.7511450 native   GAS6QKMRGS76XGKNNQ3MGWUNVQZCBLQAS2KPOITSGD6DPWQZRUQEY5BF 50.0000000 credit_alphanum4 SLT GCKA6K5PCQ6PNF5RQBF7PQDJWRHO6UOGFMRLK3DYHDOI244V47XKQ4GP true 0xc420156420}
2
{{ false} {http://localhost:8000/accounts/GBGMFKBQ6LVLDUQDROQBRA73VX7LJ3TC4MDRQAYEIPUTJU32OSILAVW4 false} {http://localhost:8000/accounts/GANTILTFDYGNWWJ67PRHTOFYF4ALXBIZ2IVGQHABNN5LBD7MXM4FCH43 false} {http://localhost:8000/operations/73664324123504641 false}}
{{{ false} { false} { false} { false}} 73664324123504641-0 73664324123504641-0 2018-04-03 03:02:45 +0000 UTC 5152692 GBGMFKBQ6LVLDUQDROQBRA73VX7LJ3TC4MDRQAYEIPUTJU32OSILAVW4 0.9999999 native   GANTILTFDYGNWWJ67PRHTOFYF4ALXBIZ2IVGQHABNN5LBD7MXM4FCH43 0.2178649 credit_alphanum4 SLT GCKA6K5PCQ6PNF5RQBF7PQDJWRHO6UOGFMRLK3DYHDOI244V47XKQ4GP false 0xc420156470}

http://localhost:8000/accounts/GBTYBQW5GLRSYMT4OJQGVAIH5KBIE5ITEWIVDZHVHIZT7XSKU6DLB6QI/payments?cursor=-213&order=asc&limit=2 ``` { "type": "https://stellar.org/horizon-errors/bad_request", "title": "Bad Request", "status": 400, "detail": "The request you sent was invalid in some way", "extras": { "invalid_field": "cursor", "reason": "the cursor -213 is a negative number: " } } ```

https://www.stellar.org/developers/horizon/reference/endpoints/trades.html http://localhost:8000/trades?limit=3&order=desc&base_asset_type=native&counter_asset_code=SLT&counter_asset_issuer=GCKA6K5PCQ6PNF5RQBF7PQDJWRHO6UOGFMRLK3DYHDOI244V47XKQ4GP&counter_asset_type=credit_alphanum4

bartekn · 2018-04-17T16:14:45Z

clients/horizon/main.go

+ offerID int64,
+ resolution int64,
+ params ...interface{},
+ ) (tradesPage TradesPage, err error)


We should probably create code-style guide for our Go code.

bartekn · 2018-04-17T16:16:45Z

clients/horizon/mocks.go

+ resolution int64,
+ params ...interface{},
+) (tradesPage TradesPage, err error) {
+ a := m.Called(baseAsset, counterAsset, offerID, resolution, params)


Should be params.... Right now, for the following params: ["a", "b", "c"] will call:

m.Called(baseAsset, counterAsset, offerID, resolution, ["a", "b", "c"])

instead of:

m.Called(baseAsset, counterAsset, offerID, resolution, "a", "b", "c")

Do you mean this?

a := m.Called(baseAsset, counterAsset, offerID, resolution, params...)

I won't get this compiled.

What errors are you getting? It should work.

This is the error message I got.

OK, I pulled your code and it seems we're falling into golang/go#18605.

The only possible workaround is to do it like:

args := []interface{}{baseAsset, counterAsset, offerID, resolution} args = append(args, params...) a := m.Called(args...) return a.Get(0).(TradesPage), a.Error(1)

which looks strange but works as expected (allows mocking params properly).

nikhilsaraf · 2018-04-17T17:32:55Z

Is the test you've listed in your comment above checked in? @michaelran16
review looks good to me. Deferring to @bartekn for review approval based on this review's history.

0xMichaelRan · 2018-04-17T20:05:57Z

Thanks, @bartekn! I started writing Go since last week, so I was assuming lots of styling guide from Java, which I then realize shouldn't be the case.

If there's no guide now, I will refer to this for now:
https://github.com/golang/go/wiki/CodeReviewComments

0xMichaelRan · 2018-04-17T20:08:33Z

@nikhilsaraf No the test code is an external project, calling horizon go API, thus not checked in.

bartekn · 2018-04-17T20:13:43Z

Thanks, @bartekn! I started writing Go since last week, so I was assuming lots of styling guide from Java, which I then realize shouldn't be the case.

No worries. I think the best solution is to connect go fmt to your code editor. Without having the one code-style we will end up with a lot of code changes connected with styling in diffs and sometimes conflicts when merging branches.

Would be great to have tests for a new method. Check main_test.go file for examples.

nikhilsaraf · 2018-04-17T20:17:54Z

+1 on the test. they should be part of the project and checked in, that way we can run them as part of the build.

0xMichaelRan · 2018-04-20T05:51:46Z

@bartekn and @nikhilsaraf
Testing code updated.

nullstyle · 2018-04-20T21:54:29Z

clients/horizon/main_test.go

@@ -266,6 +266,80 @@ var _ = Describe("Horizon", func() {
 })
 })

+ Describe("LoadTrades", func() {


Please do not add tests using gingko. As mentioned previously in slack, we should be writing our tests using the standard library's testing package and testify.

0xMichaelRan · 2018-04-25T18:05:54Z

@nullstyle I think this PR is good to go, the testing code I move to a different discussion thread: #421

nullstyle · 2018-04-26T15:40:51Z

@michaelran16 You misunderstood me: I didn't mean that you should remove the test code. I meant that you should be porting that test code into our most modern convention for go tests: the standard test library and testify.

We are doing this work such that we can remove goconvey, gingko, and gomega from our list of dependencies.

nullstyle · 2018-04-26T15:41:50Z

Whoops! I see your comment now. nvm.

0xMichaelRan · 2018-04-30T18:37:11Z

replaced with #428

0xMichaelRan added 7 commits April 12, 2018 00:04

Add HTTP 400 message in case of negative cursors

c026bae

remove redundant parenthesis

a3834fd

convert cursor using GetInt64() function

a60d921

In cursor negative, return error

0e6850c

Merge branch 'master' of github.com:stellar/go into trades_endpoint

20a3d6c

0xMichaelRan added in progress horizon labels Apr 17, 2018

0xMichaelRan self-assigned this Apr 17, 2018

0xMichaelRan requested review from nullstyle, jedmccaleb, bartekn, tomerweller and nikhilsaraf April 17, 2018 03:30

bartekn reviewed Apr 17, 2018

View reviewed changes

0xMichaelRan added 2 commits April 20, 2018 13:13

Add testing code for LoadTrades() function

0428ec8

remove comments

7f29074

nullstyle suggested changes Apr 20, 2018

View reviewed changes

0xMichaelRan added 2 commits April 25, 2018 09:24

remove test code in gingko

81b5bbe

Change args to address comments from Bartek

6341c32

0xMichaelRan mentioned this pull request Apr 30, 2018

Test loadTrades() in horizon package #428

Merged

0xMichaelRan closed this Apr 30, 2018

jedmccaleb removed the in progress label Apr 30, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Trades endpoint in Go SDK #408

Add Trades endpoint in Go SDK #408

0xMichaelRan commented Apr 17, 2018

bartekn Apr 17, 2018

bartekn Apr 17, 2018

0xMichaelRan Apr 17, 2018

bartekn Apr 17, 2018

0xMichaelRan Apr 20, 2018

bartekn Apr 20, 2018 •

edited

Loading

nikhilsaraf commented Apr 17, 2018

0xMichaelRan commented Apr 17, 2018

0xMichaelRan commented Apr 17, 2018 •

edited

Loading

bartekn commented Apr 17, 2018

nikhilsaraf commented Apr 17, 2018

0xMichaelRan commented Apr 20, 2018 •

edited

Loading

nullstyle Apr 20, 2018

0xMichaelRan commented Apr 25, 2018

nullstyle commented Apr 26, 2018

nullstyle commented Apr 26, 2018

0xMichaelRan commented Apr 30, 2018

Add Trades endpoint in Go SDK #408

Add Trades endpoint in Go SDK #408

Conversation

0xMichaelRan commented Apr 17, 2018

bartekn Apr 17, 2018

Choose a reason for hiding this comment

bartekn Apr 17, 2018

Choose a reason for hiding this comment

0xMichaelRan Apr 17, 2018

Choose a reason for hiding this comment

bartekn Apr 17, 2018

Choose a reason for hiding this comment

0xMichaelRan Apr 20, 2018

Choose a reason for hiding this comment

bartekn Apr 20, 2018 • edited Loading

Choose a reason for hiding this comment

nikhilsaraf commented Apr 17, 2018

0xMichaelRan commented Apr 17, 2018

0xMichaelRan commented Apr 17, 2018 • edited Loading

bartekn commented Apr 17, 2018

nikhilsaraf commented Apr 17, 2018

0xMichaelRan commented Apr 20, 2018 • edited Loading

nullstyle Apr 20, 2018

Choose a reason for hiding this comment

0xMichaelRan commented Apr 25, 2018

nullstyle commented Apr 26, 2018

nullstyle commented Apr 26, 2018

0xMichaelRan commented Apr 30, 2018

bartekn Apr 20, 2018 •

edited

Loading

0xMichaelRan commented Apr 17, 2018 •

edited

Loading

0xMichaelRan commented Apr 20, 2018 •

edited

Loading