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

Bug/add namespace to websocket #35

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Alescontrela
Copy link
Contributor

Modifies CryptoBook-js to conform to CryptoBook's api call syntax. Connections to the websocket are now established via a route:

sio.connect('http://0.0.0.0:5050/api/v1/real_time')

which allows us to generalize the port (in this case 5050) using nginx.

@synchronizing
Copy link
Contributor

synchronizing commented Jun 30, 2019

Modifies CryptoBook-js to conform to CryptoBook's api call syntax. Connections to the websocket are now established via a route:

sio.connect('http://0.0.0.0:5050/api/v1/real_time')

which allows us to generalize the port (in this case 5050) using nginx.

Big issue, the Cryptobook API-endpoint is /api/v1/cryptobook/<endpoint>, that's what we have been conforming too. However, I am down to have a conversation on perhaps modifying it to /api/v1/ and instead route to cryptobook with nginx on the larger structure. Also, what do you think of /api/v1/cryptobook/live or /api/v1/live?

@Alescontrela
Copy link
Contributor Author

I'm fine with either of those. It's a pretty simple change. I'll make the switch to /api/v1/cryptobook/live.

@synchronizing
Copy link
Contributor

I'm fine with either of those. It's a pretty simple change. I'll make the switch to /api/v1/cryptobook/live.

Sounds good. Feel free to merge if small change, or if need for review, lmk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants