-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Fix: Added Tracking Crypocurrencies Exchange Trades with Google Cloud Platform in Real-Time #191
Conversation
examples/cryptorealtime/src/main/java/utils/ExchangeConfiguration.java
Outdated
Show resolved
Hide resolved
examples/cryptorealtime/src/main/java/source/CryptoMarketTradeUnboundedReader.java
Show resolved
Hide resolved
#191 by dparrish and mbrukman
Changes are implemented - please review , thank you! |
Added latest review changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes. Looks good from a Python point of view now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't resolve comment threads without either explaining why it's not necessary, or pushing an update which addresses the comment. Resolving the comment thread looks like it was resolved, when it actually was not addressed at all.
Please re-review all resolved comment threads and address them, e.g., by adding copyright headers, cleaning up code, removing blank lines, etc.
examples/cryptorealtime/src/main/java/source/CryptoMarketTradeUnboundedReader.java
Outdated
Show resolved
Hide resolved
examples/cryptorealtime/src/main/java/source/CryptoMarketTradeUnboundedReader.java
Outdated
Show resolved
Hide resolved
examples/cryptorealtime/src/main/java/source/CryptoMarketTradeUnboundedReader.java
Outdated
Show resolved
Hide resolved
examples/cryptorealtime/src/main/java/source/CryptoMarketTradeUnboundedSource.java
Outdated
Show resolved
Hide resolved
java code, added license to pom.xml maven file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks for adding copyright headers everywhere! I have a few more comments which I would like to see fixed before this is finalized, but they're (relatively) minor, so I'm not going to block the merge.
examples/cryptorealtime/src/main/java/utils/ExchangeConfiguration.java
Outdated
Show resolved
Hide resolved
GoogleCloudPlatform#191 by dparrish and mbrukman
Fix: Added Tracking Crypocurrencies Exchange Trades with Google Cloud Platform in Real-Time
In this solution, we will discuss the most important design and technical decisions: i) how to set up and configure pipeline for ingesting real-time, time-series data from various crypto exchanges ii) how to design suitable data model, which facilitates querying and graphing at scale. Finally, we will provide a tutorial on how to set up and deploy the proposed architecture using GCP. By following the tutorial steps will establish a connection to multiple exchanges, subscribe to their trade feed, extract and transform these trades into a flexible format to be stored in Bigtable and to be graphed and analyzed.