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

docker-compose support & new installation method #212

Draft
wants to merge 9 commits into
base: development
Choose a base branch
from
29 changes: 0 additions & 29 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,6 @@ user_data/plot/
# Ignore Logo Folder
Rikj000 marked this conversation as resolved.
Show resolved Hide resolved
Logo/

# Ignore GitHub Secrets
GitHub-Secrets.txt

# Ignore fthypt and legacy pickle Files
*.fthypt
*.pickle

# Installer logs
pip-log.txt
pip-delete-this-directory.txt
Expand All @@ -59,25 +52,3 @@ test-results*.xml

# Ignore pytest cache files
.pytest_cache/

# Ignore freqtrade installation
.env/
build_helpers/
config_examples/
docker/
!docker/Dockerfile.MoniGoMani
docs/
freqtrade/
junit/
scripts/
tests/junit/

freqtrade.service
freqtrade.service.watchdog
freqtrade.egg-info
MANIFEST.in
pyproject.toml
pyproject.yml
setup.cfg
setup.py
setup.sh
3 changes: 3 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[submodule "freqtrade"]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fan of the Freqtrade repo submodule!
Didn't knew that was possible with git but it seems to be perfect for embedding the Freqtrade repo in the MoniGoMani repo in a clean way! 🎉

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a great feature, but it must be used with caution (it shouldn't be abused). I think this is an excellent example of when to use it! I'm glad this was relevant :)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'll have to take a good in-depth read in the docs on how to manage submodules once we're about to merge this PR!
Have to make sure I know how to use it right so I don't accidentally update the submodule without it being my intention 😄

path = freqtrade
url = git@github.com:freqtrade/freqtrade.git
7 changes: 1 addition & 6 deletions .hurry
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
config:
username: MoniGoMani Community
exchange: binance
ft_binary: source ./.env/bin/activate; freqtrade
Copy link
Owner

@Rikj000 Rikj000 Dec 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with deprecating ft_binary as long as it doesn't deprecate the ability to use the mgm-hurry command from everywhere when a shell alias has been set!

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the implementation of PR #224 we should easily be able to deprecate the ft_binary now 🙂

install_type: source
hyperopt:
epochs: 1000
loss: MGM_WinRatioAndProfitRatioHyperOptLoss
stake_currency: USDT
spaces: buy sell
strategy: MoniGoManiHyperStrategy
install_type: source
mgm_config_names:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why we would want to deprecate the mgm_config_names section in .hurry?

This currently gives users the ability to easily switch between config files that co-exist in the user_data folder,
examples could be mgm-config-usdt.json & mgm-config-ust.json or mgm-config-private-binance.json & mgm-config-private-kucoin.json.
That's a nice feature to have in my opinion.

I would even agree about moving all .hurry setting besides the mgm_config_names section & install_type from .hurry => mgm-config,
That way we would unify the settings which users configure manually often more which should lead to less confusion about where certain settings are.
(Most users don't know their username can be altered in .hurry for example)

The new proposed "minimal" .hurry format would be of following format then:

config:
  install_type: source
  mgm_config_names:
    mgm-config: mgm-config.json
    mgm-config-hyperopt: mgm-config-hyperopt.json
    mgm-config-private: mgm-config-private.json

(Then we can also scrap the double exchange & stake_currency settings, which currently are defined in both mgm-config & .hurry)
However I suggest we keep this for a separate issue!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to be a bit controversial but I hope it's open for discussion. Allow me to elaborate my reasons to make that decision as well as to cover on this reply your other comment with my reasons to add the new command use_configuration to the framework.

The main reason on to why I had to deprecate the mgm_config_names from the .hurry file was because Docker can't access the .hurry file. In fact, the only directory FQ has access to are the contents of the user_data directory, which is part of the I/O files that FQ requires to run. If you look at the docker-compose file (from FQ instructions) you'll notice that Docker will mount a volume to access the strategy files. One could add more volumes to the container, but it'll just make more complicated architecture IMHO.

My proposal:

  • Follow similar paradigm that FQ uses. Make the strategy folder user_data static, on that directory and simply load whatever is there.
  • Change the process when the user needs to "install" a new configuration, located somewhere on his FS, by using the new use_configuration command.

My goal with this approach is to clean-up what I consider FQ files, from what I consider files for the MGM framework, then temporary files/output and the (possible several) user's strategy configuration files. This makes sense if you use a git repo, or a external folder somewhere in your FS, to store your strategy config files without polluting the MGM git repo. My goal for a clean process is: clone the official repo, install, configure framework with my config files and then run. Placing all my data inside the user_data folder doesn't allow me to store the config in a git repo without moving them around each time I modify them.

Following your example from your comment, the user will have a folder my_mgm_configurations anywhere and subfolders that contain all files that make a strategy (the config files!). And this is swapped by the use_cofiguration command. Does that make sense?

To be honest, I see the .hurry more like a "environment configuration file" where you define how you've installed FQ or other meta stuff that is automatically access by MGM framework. But not things that are related to a certain configuration, that belongs to the config files IMO and that can be swapped easily using the use_configuration command.

Copy link
Owner

@Rikj000 Rikj000 Dec 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do agree with .hurry being an "environment configuration file",
that's also why I proposed the new "minimal" .hurry format above which would remove all Strategy/Framework related config settings from it.
And then move them to the main mgm-config file where they would be placed more appropriately! 🙂

And if it's about .hurry & mounting with docker then I propose one of 2 following changes:

  • Move .hurry from the root Freqtrade-MGM/ folder to Freqtrade-MGM/user_data/,
    that way it would be accessible by docker since it'll be in the existing volume.
    (Leaning more towards this option since user_data/ is Freqtrade's official folder to store used config files, so it would group the config files in a more logical way)
  • Create a new volume for .hurry in our docker-compose.yml,
    that way it also would be accessibly by docker, but as you mentioned this complicates the docker-compose.yml some more.

However I'm a bit in disagreement with the scrapping the ability to configure custom config names (aka the mgm_config_names section in .hurry),
and the use_configuration implementation that you propose. Since I believe it will lead to more confusion from an end user perspective.

Allow me to elaborate the concerns I have:

  • user_data/ is Freqtrade's official folder to store used config files,
    Freqtrade users will be used to it & look for their config files in that folder instead of user_data/my_mgm_configurations/
  • Imagine a user has following config files stored under user_data/my_mgm_configurations/:
    • mgm-config-usdt.json

    • mgm-config-ust.json

    • mgm-config-tusd.json

    • mgm-config-private-binance.json

    • mgm-config-private-kucoin.json

      Then the user executes mgm-hurry use_configuration and picks mgm-config-usdt.json & mgm-config-private-binance.json,
      they are copied over to user_data/ and renamed mgm-config.json & mgm-config-private.json.

      Imagine that at this point the user doesn't use MGM for a few months & comes back,
      however they forget which files they picked when using mgm-hurry use_configuration and since this implementation renamed the config files,
      they can't find out anymore which config files of user_data/my_mgm_configurations/ are currently in use without opening the files & comparing them against each other with a diff checker. Many end users won't know how to do that.

  • Imagine the user has config changes in mgm-config.json and then executes mgm-hurry use_configuration,
    this will lead to mgm-config.json being overwritten and might lead to lost config changes in some occasions,
    unless we implement extra logic to prompt the user if he wants to backup his current config files under a new name before overwriting.

That's why I'd like to keep the mgm_config_names section in .hurry since compared to the solution you suggest it'll have following benefits:

  • user_data/ is Freqtrade's official folder to store used config files, so more users will find the config files without troubles
  • Imagine a user has following config files stored under user_data/:
    • .hurry (Assuming we'll roll with moving .hurry to user_data/)

    • mgm-config-usdt.json

    • mgm-config-ust.json

    • mgm-config-tusd.json

    • mgm-config-private-binance.json

    • mgm-config-private-kucoin.json

      Then the user executes mgm-hurry switch_configuration and picks mgm-config-usdt.json & mgm-config-private-binance.json,
      the names will be stored in .hurry under mgm_config_names.

      Imagine that at this point the user doesn't use MGM for a few months & comes back,
      and again they forgot which config files they picked. Now they can easily check the contents of .hurry to find out which config files are currently in use 🙂

  • Imagine the user is using and has config changes in mgm-config-usdt.json, then executes mgm-hurry switch_configuration and picks mgm-config-ust.json.
    This will simply switch the used configuration file without any overrides being done, so there also won't be any chance for losing config changes!

I really do appreciate thorough thinking through that you're doing for this issue here!
But I hope you can also agree upon my proposal after clarifying my concerns with your suggestion. 🙂

mgm-config: mgm-config.json
mgm-config-hyperopt: mgm-config-hyperopt.json
mgm-config-private: mgm-config-private.json
timerange: 20210501-20210616
30 changes: 30 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
---
version: '3'
services:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I right that we're currently only dockerizing freqtrade & not yet mgm-hurry itself?
Or will docker build -t fq:mgm-recommended . build a container containing both based on the local folder structure?

I'm also wondering if mgm-hurry install_freqtrade & mgm-hurry install_mgm will be able to update their own docker containers?

Or will docker users be forced to re-use the installer.sh script for updating?
(Which normally was only intended for an initial web-installation, after that users update through mgm-hurry itself)

Copy link
Author

@Bloodsucker Bloodsucker Dec 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I right that we're currently only dockerizing freqtrade & not yet mgm-hurry itself?
Or will docker build -t fq:mgm-recommended . build a container containing both based on the local folder structure?

Currently only FQ is dockerized but it's possible to run both MGM and FQ inside their own containers (each on their own image). Docker-composer allows to easily manage a set of containers and run "two services" with dependencies and thanks to the Docker magic communicate with each other. It's possible but I've never done it myself.

I'm also wondering if mgm-hurry install_freqtrade & mgm-hurry install_mgm will be able to update their own docker containers?

To update to a newer MGM version, the MGM framework commands should simply checkout the MGM git commit (which will also update the FQ submodule) or checkout a new commit/tag (tag, please!) in the the FQ submodule. Then build. Both Dockerized or source code installation will be automatically updated.

Or will docker users be forced to re-use the installer.sh script for updating?
(Which normally was only intended for an initial web-installation, after that users update through mgm-hurry itself)

This is a good question, but if they already have a working MGM the commands should just checkout to a newer commit version and then build. No need to run external scripts IMO.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I right that we're currently only dockerizing freqtrade & not yet mgm-hurry itself?
Or will docker build -t fq:mgm-recommended . build a container containing both based on the local folder structure?

Currently only FQ is dockerized but it's possible to run both MGM and FQ inside their own containers (each on their own image). Docker-composer allows to easily manage a set of containers and run "two services" with dependencies and thanks to the Docker magic communicate with each other. It's possible but I've never done it myself.

I'm a fan of implementing 2 separate services in the docker-compose.yml, it would be a cleaner way of doing things!
I've recently done this with Flagsmith, where they deploy a postgres service for the DataBase and a flagsmith service for the front-end which is connected to the postgres service.
Perhaps we can use their docker-compose.yml as an example.

However I do fear that it might complicate the implementation since we'll have to make sure that the mgm-hurry service will be able to run commands from the freqtrade service without any issues.
Which wouldn't be an issue if we simply run both mgm-hurry and freqtrade in a single container service.

I'm also wondering if mgm-hurry install_freqtrade & mgm-hurry install_mgm will be able to update their own docker containers?

To update to a newer MGM version, the MGM framework commands should simply checkout the MGM git commit (which will also update the FQ submodule) or checkout a new commit/tag (tag, please!) in the the FQ submodule. Then build. Both Dockerized or source code installation will be automatically updated.

Hmmm I'm not such a fan of automatically updating Freqtrade when you update MGM 🧐
Of course most of the times it'll be the recommended thing to do!
(If MGM updates Freqtrade, then the end user will probably have to update his Freqtrade too to remain compatibility)

However this can also lead to confusion for the end user since it won't be clear that his Freqtrade also updated when he updated MGM.

So I propose the following when updating MGM:

  • Temporarily store the Freqtrade tag/commit used
  • Update MGM (which will automatically update Freqtrade)
  • Check if the Freqtrade tag/commit was updated during the installation
    • If the Freqtrade tag/commit was updated, prompt the user if he'd like to update to the newer Freqtrade recommended by MGM
      • If the answer was yes, finish installation (Freqtrade was already updated when updating MGM)
      • If the answer was no, roll Freqtrade back to the temporarily stored Freqtrade tag/commit at the 1st step

Or will docker users be forced to re-use the installer.sh script for updating?
(Which normally was only intended for an initial web-installation, after that users update through mgm-hurry itself)

This is a good question, but if they already have a working MGM the commands should just checkout to a newer commit version and then build. No need to run external scripts IMO.

In my answers above I assumed that the docker containers will be able to run the mgm-hurry install_freqtrade & mgm-hurry install_mgm commands.
However I do have a concern that they won't have enough permission to alter the contents of the whole local MGM repo.

As we've talked about before, currently the docker-compose.yml provided by Freqtrade only mounts the user_data/ folder as an accessible volume, but we might have to mount the whole MGM repo folder as a volume to be able to do this.

I'm not certain if this will or won't be needed though, so it'll have to be tested.

freqtrade:
image: fq:mgm-recommended
# image: freqtradeorg/freqtrade:stable
# image: freqtradeorg/freqtrade:develop
# Use plotting image
# image: freqtradeorg/freqtrade:develop_plot
# Build step - only needed when additional dependencies are needed
# build:
# context: .
# dockerfile: "./docker/Dockerfile.custom"
restart: unless-stopped
container_name: freqtrade
volumes:
- "./user_data:/freqtrade/user_data"
# Expose api on port 8080 (localhost only)
# Please read the https://www.freqtrade.io/en/stable/rest-api/ documentation
# before enabling this.
ports:
# - "127.0.0.1:8080:8080"
- "127.0.0.1:8080:8080" # Unsafe
# Default command used when running `docker compose up`
command: >
trade
--logfile /freqtrade/user_data/logs/freqtrade.log
--db-url sqlite:////freqtrade/user_data/tradesv3.sqlite
--config /freqtrade/user_data/config.json
--strategy SampleStrategy
1 change: 1 addition & 0 deletions freqtrade
Submodule freqtrade added at 3503fd
87 changes: 61 additions & 26 deletions mgm-hurry
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import glob
import json
import logging
import os
import shutil
import sys
from datetime import datetime
from string import Template
Expand Down Expand Up @@ -69,6 +70,7 @@ class MGMHurry:
self.freqtrade_cli = FreqtradeCli(self.basedir)
self.monigomani_cli = MoniGoManiCli(self.basedir)

# TODO redo.
def version(self):
if self.freqtrade_cli.install_type == 'source':
if self.freqtrade_cli.installation_exists(silent=True):
Expand All @@ -82,7 +84,6 @@ class MGMHurry:
self.monigomani_cli.run_command('git log -1; echo "";')
else:
self.logger.warning(Color.yellow('"No Freqtrade installation detected!'))

else:
self.logger.warning(Color.yellow('"version" command currently only supported on "source" installations.'))

Expand Down Expand Up @@ -577,8 +578,9 @@ class MGMHurry:

self.logger.info(f'👉 Downloading candle data ({tickers}) for timerange ({timerange})')

command = (f'{self.monigomani_config.config["ft_binary"]} download-data --timerange {timerange} '
f'-t {tickers} {self.monigomani_config.command_configs()}')
command = f'{self.monigomani_config.get_freqtrade_cmd()} download-data'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

f-strings can be appended to each other without command +=.
A cleaner implementation would be:

command = (f'{self.monigomani_config.get_freqtrade_cmd()} download-data'
           f' --timerange {timerange} -t {tickers} {self.monigomani_config.command_configs()}')

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually would have prefer this style:

command = ' '.join([
  self.monigomani_config.config["ft_binary"],
  'download-data',
  f'--timerange {timerange}',
  f'-t {tickers}',
  self.monigomani_config.command_configs()
])

I believe this is a discussion over a personal style, however I will honor yours since you're the creator and maintainer.

Copy link
Owner

@Rikj000 Rikj000 Dec 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's indeed a discussion of style,
I would agree upon your style suggestion for smaller projects, because it does give a very clean code readability!
And as a + it also eliminates the chance of getting a space too little or too many in the commands. 👏

However I appreciate you sticking to my suggestion, for MGM I prefer this writing because it's already becoming a rather large project & this will keep the amount of lines significantly lower while code readability remains okay 🙂
(We check with flake8 against line lengths not going too long)

command += f' --timerange {timerange}'
command += f' -t {tickers} {self.monigomani_config.command_configs()}'

if self.monigomani_config.config['exchange'] == 'kraken':
command += ' --dl-trades'
Expand Down Expand Up @@ -698,30 +700,24 @@ class MGMHurry:
if spaces is None:
spaces = self.monigomani_config.config['hyperopt']['spaces']

command = (f'$ft_binary hyperopt -s $ho_strategy {self.monigomani_config.command_configs()}'
f'--hyperopt-loss $ho_loss --spaces $ho_spaces -e $ho_epochs --timerange $timerange ')
command = f'{self.monigomani_config.get_freqtrade_cmd()} hyperopt'
command += f' {self.monigomani_config.command_configs()}'
command += f' --hyperopt-loss {strategy}'
command += f' --spaces {spaces}'
command += f' -e {epochs}'
command += f' --timerange {timerange}'

if enable_protections is True:
command = f'{command.strip()} --enable-protections '
command = f' {command.strip()} --enable-protections'
if random_state is not None:
command = f'{command.strip()} --random-state $random_state '
command = f' {command.strip()} --random-state {random_state}'
if jobs is not None:
command = f'{command.strip()} -j $jobs '
command = f' {command.strip()} -j {jobs}'
if min_trades is not None:
command = f'{command.strip()} --min-trades $min_trades '

command = Template(command).substitute(
ft_binary=self.monigomani_config.config['ft_binary'],
ho_strategy=strategy,
ho_loss=loss,
ho_spaces=spaces,
ho_epochs=epochs,
timerange=timerange,
random_state=random_state,
jobs=jobs,
min_trades=min_trades)
command = f' {command.strip()} --min-trades {min_trades}'

self.logger.debug(command)

if output_file_name is None:
output_file_name = f'{strategy}-{datetime.now().strftime("%Y-%m-%d_%H-%M-%S")}'
hyperopt_file_name = f'HyperOptResults-{output_file_name}'
Expand Down Expand Up @@ -794,7 +790,7 @@ class MGMHurry:
best = '--best' if only_best is True else ''
profit = '--profitable' if only_profitable is True else ''

command = (f'{self.monigomani_config.config["ft_binary"]} hyperopt-list '
command = (f'{self.monigomani_config.get_freqtrade_cmd()} hyperopt-list '
f'--hyperopt-filename "{choice}" {best} {profit}')
self.logger.debug(command)

Expand Down Expand Up @@ -823,7 +819,7 @@ class MGMHurry:

self.logger.info(f'👉 Showing {"" if apply is False else "and applying "}HyperOpt results for epoch #{epoch}')

command = (f'{self.monigomani_config.config["ft_binary"]} hyperopt-show -n {epoch} '
command = (f'{self.monigomani_config.get_freqtrade_cmd()} hyperopt-show -n {epoch} '
f'{self.monigomani_config.command_configs()}')

if fthypt_name is not None:
Expand Down Expand Up @@ -857,7 +853,7 @@ class MGMHurry:
self.logger.info(Color.title('👉 Start BackTesting. Lets see how it all turns out!'))

timerange = self.monigomani_config.get_preset_timerange(timerange)
command = (f'$ft_binary backtesting -s $ho_strategy {self.monigomani_config.command_configs()}'
command = (f'{self.monigomani_config.get_freqtrade_cmd()} backtesting -s $ho_strategy {self.monigomani_config.command_configs()}'
f'--timerange $timerange')

if timerange is None:
Expand Down Expand Up @@ -950,7 +946,7 @@ class MGMHurry:
output_file_path = f'{self.basedir}/user_data/plot/{plot_profit_file_name}.html'

self.monigomani_cli.run_command(
f'{self.monigomani_config.config["ft_binary"]} plot-profit {self.monigomani_config.command_configs()}'
f'{self.monigomani_config.get_freqtrade_cmd()} plot-profit {self.monigomani_config.command_configs()}'
f'--export-filename {backtest_results_path} --timerange {timerange} --timeframe {timeframe} && '
f'mv {self.basedir}/user_data/plot/freqtrade-profit-plot.html {output_file_path}')

Expand Down Expand Up @@ -1098,6 +1094,44 @@ class MGMHurry:
message=f'🚀 Fresh **{strategy}** SpreadSheet (.csv) Results ⬇️',
results_paths=[output_file_path])

def use_configuration(self, config_ho: str = None, config: str = None, config_private: str = None, override: bool = False) -> None:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do like the idea for the new mgm-hurry use_configuration command!

However I would do the implementation of it quite differently:

  • Rename the command to mgm-hurry switch_configuration
  • Remove the mandatory --override modifier (Default intention would be to simply switch between existing configs instead of overriding)
  • Add an optional --new modifier:
    • Defaults to False
    • If True is passed, allow creation of new or overriding existing mgm-config & mgm-config-private files based on mgm-config.example & mgm-config-private.example,
      while skipping the interactive list prompts by passing a config-name
  • For the functionality of the other 3 optional modifiers (--config, --config_ho & --config_private):
    • Don't pass the full path, only pass the config-name (With or without .json, append .json if not provided by user), since all config files will exist in user_data
    • If nothing is passed, launch an interactive list prompt to allow picking of .json file existing in user_data + additional new config file option.
      • Selecting new config file launches an interactive text prompt, allowing you to fill in a config name (With or without .json, append .json if not provided by user)
    • If a config-name is passed without --new in the command,
      check if existing & automatically use (skip the corresponding interactive list prompt), if not existing also launch interactive list prompt.
    • If a config-name is passed with--new in the command, automatically use (skip the corresponding interactive list prompt)
    • If --new is passed or new config file was selected, check if config file already exists:
      • Create config-name config file with a new copy of mgm-config.example or mgm-config-private.example if not existing
      • Warn with interactive yes/no prompt if it's okay to overwrite the existing config file with a new copy of mgm-config.example or mgm-config-private.example
    • Update corresponding config name in the mgm_config_names section of .hurry
    • Wildcard names for these files would be very nice since it would allow us to auto filter the interactive list prompts!
      Then we can make it so they only show the correct config file types in their selection, some examples:
      • All new mgm-config files will be pre-fixed with mgm-config-, example: user fills in usdt, new file will be called mgm-config-usdt.json
      • All new mgm-config-private files will be pre-fixed with mgm-config-private-, example: user fills in kucoin.json, new file will be called mgm-config-private-kucoin.json
      • All new mgm-config-hyperopt files will be pre-fixed with mgm-config-hyperopt-, example: user fills in ust-binance, new file will be called mgm-config-hyperopt-ust-binance.json

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please refer to the other (very) long reply: #212 (comment)

I like your proposal of renaming the command, but the command makes sense if different the configuration files aren't directly in the user_data folder. Refer to the other comment for more info on this.

"""
Configure the bot with the given configuration. Use this to quickly move between different configurations
stored in your filesystem.

:param config_ho: (str)
:param config: (str, Optional)
:param config_private: (str, Optional)
:param override: (bool)
"""
self.logger.info(Color.title('👉 Configuring the bot with strategy:'))

if not override:
self.logger.error('Impossible to use provided configuration without overridden the present one.'
' Include the `--override` flag if you wish to override the current configuration.')
return

if os.path.isfile(config_ho):
self.logger.info(f'Using config-hyperopt from: {config_ho}')

shutil.copy(config_ho, f'{self.basedir}/user_data/mgm-config-hyperopt.json')
elif config_ho is not None:
self.logger.error(f'Can\'t locate config-hyperopt file from: {config_ho}')

if os.path.isfile(config):
self.logger.info(f'Using config from: {config}')

shutil.copy(config, f'{self.basedir}/user_data/mgm-config.json')
elif config is not None:
self.logger.error(f'Can\'t locate config file from: {config}')

if os.path.isfile(config_private):
self.logger.info(f'Using config-private from: {config_private}')

shutil.copy(config_private, f'{self.basedir}/user_data/mgm-config-private.json')
elif config_private is not None:
self.logger.error(f'Can\'t locate config-private file from: {config_private}')

def start_trader(self, dry_run: bool = True):
"""
Start the trader. Your ultimate goal!
Expand All @@ -1107,8 +1141,9 @@ class MGMHurry:
self.logger.info(Color.title(f'👉 Starting the trader, '
f'{"in Dry-Run Mode!" if dry_run else "in Live-Run Mode, fingers crossed! 🤞"}'))

command = (f'{self.monigomani_config.config["ft_binary"]} trade {self.monigomani_config.command_configs()}'
f'--strategy {self.monigomani_config.config["hyperopt"]["strategy"]}')
command = f'{self.monigomani_config.get_freqtrade_cmd()} trade'
command += f' {self.monigomani_config.command_configs()}'
command += f' --strategy {self.monigomani_config.config["hyperopt"]["strategy"]}'

if dry_run is True:
command += ' --dry-run'
Expand Down
2 changes: 2 additions & 0 deletions requirements-mgm-dev.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-r requirements-mgm.txt
pytest==6.2.5
28 changes: 19 additions & 9 deletions user_data/mgm_tools/mgm_hurry/FreqtradeCli.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# \_| |_| \___| \__, | \__||_| \__,_| \__,_| \___| \____/|_||_|
# | |
# |_|
import shutil

import distro
import glob
Expand Down Expand Up @@ -129,27 +130,36 @@ def installation_exists(self, silent: bool = False) -> bool:
return False

# Well if install_type is docker, we return True because we don't verify if docker is installed
if self.install_type == 'docker':
if self.install_type == 'docker-compose':
if silent is False:
self.cli_logger.debug('FreqtradeCli - installation_exists() succeeded because '
'install_type is set to docker.')
return True

if self.freqtrade_binary is None:
if silent is False:
self.cli_logger.warning(Color.yellow('FreqtradeCli - installation_exists() failed. '
'No freqtrade_binary.'))
return False

if self.install_type == 'source':
if silent is False:
self.cli_logger.debug('FreqtradeCli - installation_exists() install_type is "source".')
if os.path.exists(f'{self.basedir}/.env/bin/freqtrade'):

if os.path.isfile('freqtrade/.env/bin/freqtrade'):
return True

if silent is False:
self.cli_logger.warning(Color.yellow(f'FreqtradeCli - installation_exists() failed. Freqtrade binary '
f'not found in {self.basedir}/.env/bin/freqtrade.'))
f'not found.'))

if self.install_type == 'custom' and self.freqtrade_binary:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we only log here, we can replace these 3 if silent is False: checks with a single if silent is False: check.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Care to explain how does the silent mode works? Thanks!

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optional silent parameter can be used to check if Freqtrade and/or MGM are installed through the self.freqtrade_cli.installation_exists(silent=True) and self.monigomani_cli.installation_exists(silent=True) functions without logging to the user about it.

By default these functions would log output to the user which is not always what we want 🙂
For example in the mgm-hurry up command:
mgm-hurry-silent

if os.path.isfile(self.freqtrade_binary):
if silent is False:
self.cli_logger.debug('FreqtradeCli - installation_exists() succeeded because '
'install_type is set to custom.')
else:
if silent is False:
self.cli_logger.warning(Color.yellow(f'FreqtradeCli - installation_exists() failed. Freqtrade binary '
f'not found.'))

if silent is False:
self.cli_logger.warning(Color.yellow('FreqtradeCli - installation_exists() failed. '
'No freqtrade_binary.'))

return False

Expand Down
19 changes: 15 additions & 4 deletions user_data/mgm_tools/mgm_hurry/MoniGoManiConfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,18 @@ def read_hurry_config(self) -> dict:

return hurry_config

def get_freqtrade_cmd(self):
if self.config['install_type'] == 'docker-compose':
cmd = 'docker-compose run --rm freqtrade'

return cmd
elif self.config['install_type'] == 'source':
cmd = f'{self.basedir}/freqtrade/.env/bin/freqtrade'

return cmd
elif self.config['install_type'] == 'custom':
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious which other custom installation types you can think off?
If none then we can perhaps simply drop custom installation types? 🙂

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah... the use isn't really clear 😃 . However, I wanted to use it as a replacement for the .hurry install_fq. Think on a situation where the environment is customized by the user. E.g. FQ is installed in a different location in the system and it doesn't follow the recommended/supported/default MGM framework installation. If removed, with the current changes the user wouldn't be able to customize FQ command with its own.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in the end the process of installation will be very similar for the end user as it currently is:

  • Do the initial installation through the web installer with:
    /usr/bin/env sh <(curl -s "https://github.com/raw/Rikj000/MoniGoMani/development/installer.sh")
  • Do further updates with mgm-hurry install_freqtrade & mgm-hurry install_mgm
    (Now that I think more about this, perhaps we should rename the commands to mgm-hurry update_freqtrade & mgm-hurry update_mgm, since they will only be used once for installation (Automatically called through mgm-hurry up in the web-installer). But they will always be used for updating after that, by directly calling them as mgm-hurry ...)

Because the web installer allows for installation in a custom folder & with a custom installation folder name I think we should be able to drop the custom installation type 🙂

I'm still fine with dropping the ft_binary as long as MGM won't have issues detecting the current working directory without it.
Since users can simply run the web installer once again if they wish to move/rename their installation.

raise "TODO install_type 'custom' unsupported"

def get_config_filepath(self, cfg_key: str) -> str:
"""
Transforms given cfg_key into the corresponding absolute config filepath.
Expand Down Expand Up @@ -476,13 +488,12 @@ def save_weak_strong_signal_overrides(self) -> bool:

def command_configs(self) -> str:
"""
Returns a string with the 'mgm-config' & 'mgm-config-private' names loaded from '.hurry'
Returns a string with the 'mgm-config' & 'mgm-config-private' file paths
ready to implement in a freqtrade command.
:return str: String with 'mgm-config' & 'mgm-config-private' for a freqtrade command
"""
mgm_json_name = self.config['mgm_config_names']['mgm-config']
mgm_private_json_name = self.config['mgm_config_names']['mgm-config-private']
return f'-c ./user_data/{mgm_json_name} -c ./user_data/{mgm_private_json_name} '

return '-c ./user_data/mgm-config.json -c ./user_data/mgm-config-private.json'

def get_preset_timerange(self, timerange: str) -> str:
"""
Expand Down
Loading