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

fix: refactor tradingview #383

Merged
merged 3 commits into from
Nov 2, 2021
Merged

fix: refactor tradingview #383

merged 3 commits into from
Nov 2, 2021

Conversation

chrisleekr
Copy link
Owner

@chrisleekr chrisleekr commented Oct 31, 2021

Description

The TradingView container was developed with a very simple python web server.
However, it was found that TradingView container was causing some issues with some scenarios.

  • When a symbol is not supported by TradingView
  • When requests were taking a long time.

To resolve the issue, this PR has refactored TradingView container with Flask.

How to test:

$ git pull
$ git checkout fix/refactor-tradingview
$ npm run docker:build
$ docker-compose -f docker-compose.server.yml up -d
$ docker ps -a
$ docker logs binance-bot -f --tail=1000 | bunyan

Related Issue

#368 #381

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

@codecov-commenter
Copy link

codecov-commenter commented Oct 31, 2021

Codecov Report

Merging #383 (b62200c) into master (f51adfc) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #383   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           80        80           
  Lines         2936      2936           
  Branches       546       546           
=========================================
  Hits          2936      2936           
Impacted Files Coverage Δ
...ob/trailingTradeIndicator/step/get-trading-view.js 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa9e9ff...b62200c. Read the comment docs.

@Rayn0r
Copy link

Rayn0r commented Oct 31, 2021

I checked out fix/refactor-tradingview and had the docker images built.
Things look promising after docker-compose ... -up. The Logs show TV data again:
image

@chrisleekr
Copy link
Owner Author

That is good. Try to run a few days to see how it goes.

@Rayn0r
Copy link

Rayn0r commented Oct 31, 2021

My results should be taken with a grain of salt, since I'm still running Ubuntu 18.04 and docker version 3.7 is not yet supported (I put 3.3 on the docker.yml )and even npm is warning about:

npm WARN read-shrinkwrap This version of npm is compatible with lockfileVersion@1, but package-lock.json was generated for lockfileVersion@2. I'll try to do my best with it!

But I'll let it run for a few days and see how it performs.
Perhaps @thesam1545 or @atol71 could run a test as well.

@chrisleekr
Copy link
Owner Author

@Rayn0r I am running Ubuntu 18.04 as well

$ lsb_release  -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 18.04.5 LTS
Release:	18.04
Codename:	bionic

And I don't have an issue running docker compose 3.7

My docker-compose version is

$ docker-compose -v
docker-compose version 1.26.2, build eefe0d31

@chrisleekr chrisleekr merged commit 7f12028 into master Nov 2, 2021
@chrisleekr chrisleekr deleted the fix/refactor-tradingview branch November 9, 2021 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No trading view after login on server.yml TrandingView signals do not updating
3 participants