Skip to content
This repository has been archived by the owner on Jun 8, 2020. It is now read-only.

[bitfinex] Added some methods of authenticated api (version 2) #273

Merged
merged 9 commits into from
Jan 29, 2019

Conversation

badgerwithagun
Copy link
Collaborator

I have taken the code in #209 and made a few modifications to bring it in line with other exchanges where we have/are implementing authenticated support:

  • Use the same web socket for authenticated and unauthenticated data, and connect them both by default
  • Use the standard means of supplying authentication details, via ExchangeSpecification
  • Expose the new Observable methods on the market data service, in line with [Binance] Support for execution reports / UserTrades / fills #246 and to make it ready for exposure through the generic API I'm working on.
  • Fix a number of review issues

I'm going to be testing this over the next week or so. @davidjirovec - I'd be grateful if you could try out this version and see if it works for you as well as the original.

@davidjirovec
Copy link
Contributor

@davidjirovec - I'd be grateful if you could try out this version and see if it works for you as well as the original.

Thanks. I hope I will have time soon.

@badgerwithagun
Copy link
Collaborator Author

See #274 for some context around the approach used and my plans for where to take this next

@badgerwithagun
Copy link
Collaborator Author

Hmmm, this is confusing. It seems that xchange-stream uses the v2 socket API but is built over the v1 REST API. :/

@davidjirovec
Copy link
Contributor

I've deployed this version, let's keep it running at least for a day :)

One thing to consider: I would like to log all traffic from authenticated channels, but not public channels.

In the previous PR, traffic from authenticated channels was flowing through BitfinexStreamingRawService instead of regular BitfinexStreamingService, which made it easy to distinguish in logger configuration.

Now all traffic is going through one BitfinexStreamingService, which I agree is correct solution, but it makes logging more complicated. For now I've set BitfinexStreamingAdapters logging to debug, but this would log just known and parsed messages.

@badgerwithagun
Copy link
Collaborator Author

Do you need to log the raw socket data, or further downstream (e.g. after transformation into XChange objects)?

@davidjirovec
Copy link
Contributor

Actually both, but for xchange objects enabling debug on BitfinexStreamingAdapters seems to do it.

Now I am missing raw data.

@badgerwithagun
Copy link
Collaborator Author

It would be a simple PR to use multiple loggers in the streaming service - perhaps append the channel name or message type to the logger name?

This sort of thing is pretty common.

@davidjirovec
Copy link
Contributor

It would be a simple PR to use multiple loggers in the streaming service - perhaps append the channel name or message type to the logger name?

Sounds good.

Otherwise testing seems fine.

@badgerwithagun
Copy link
Collaborator Author

Thanks @davidjirovec. I'll merge this later this evening.

@badgerwithagun
Copy link
Collaborator Author

Ah, darnit, needs a second reviewer with merge access. Sorry to pester you again @Flemingjp ...

@Flemingjp
Copy link
Collaborator

@badgerwithagun No worries - that's what email notifications are for ;)

@Flemingjp Flemingjp merged commit 85c1f7f into bitrich-info:develop Jan 29, 2019
@badgerwithagun badgerwithagun deleted the bitfinex-authenticated branch February 4, 2019 12:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants