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

⚡️ Rename to .example.json config files and .gitignore original files #74

Closed
rvasilev opened this issue Jun 23, 2021 · 5 comments
Closed
Assignees
Labels
Feature - Enhancement Update or improvement to existing feature
Milestone

Comments

@rvasilev
Copy link
Contributor

So mgm-config could be left alone with best recommended vanilla / unchanged settings and most of the tuning will be done via mgm-config-private.

####################################################################################################################
# START OF CONFIG NAMES SECTION #
####################################################################################################################
mgm_config_name = 'mgm-config.json'
mgm_config_hyperopt_name = 'mgm-config-hyperopt.json'
####################################################################################################################
# END OF CONFIG NAMES SECTION #
####################################################################################################################
# MGM trend names
mgm_trends = ['downwards', 'sideways', 'upwards']
# Initialize empty buy/sell_params dictionaries and initial (trailing)stoploss values
buy_params = {}
sell_params = {}
# Load the MoniGoMani settings
mgm_config_path = os.getcwd() + '/user_data/' + mgm_config_name
if os.path.isfile(mgm_config_path) is True:
# Load the 'mgm-config.json' file as an object and parse it as a dictionary
file_object = open(mgm_config_path, )
json_data = json.load(file_object)
mgm_config = json_data['monigomani_settings']
else:
sys.exit(f'MoniGoManiHyperStrategy - ERROR - The main MoniGoMani configuration file ({mgm_config_name}) can\'t '
f'be found at: {mgm_config_path}... Please provide the correct file and/or alter "mgm_config_name" in '
f'"MoniGoManiHyperStrategy.py"')

Most probably the logical and almost drop-in replacement way to implement is freqtrade.configuration.Configuration.from_files() .

class Configuration:
    """
    Class to read and init the bot configuration
    Reuse this class for the bot, backtesting, hyperopt and every script that required configuration
    """
@rvasilev rvasilev changed the title Overridable mgm-config with mgm-config-private / change MGM configuration loading logic Overridable mgm-config with mgm-config-private / change MGM configuration loading logic Jun 23, 2021
@Rikj000 Rikj000 added Question Further information is requested Won't Fix This will not be worked on labels Jun 23, 2021
@Rikj000
Copy link
Owner

Rikj000 commented Jun 23, 2021

Sorry man but absolutely not! mgm-config-private.json should contain no settings related to configuration!
This file is only meant for delicate private information, such as usernames, passwords, keys, domain names, tokens and such.
All actual settings belong in mgm-config.json and if you need the latest official again then you can just grab it from GitHub or through git 🙂

We do this to keep mgm-config.json setups easy to share and to reduce the risk of someone leaking out his private information!
If you would accidentally post your current mgm-config-private.json then that would be very bad for you, since then with access to the file, can use it to access your Exchange API / wallet!

Also I don't see what's wrong with MGM's current config loading? It's not long? The setting of the class parameters after the json loading is rather lengthy though. There I think we could perhaps improve, but we would need to be very carefully that everything keeps initializing as expected.

FYI: I tried Freqtrades config accessing through the dataprovider but that doesn't work since we already need it while MasterMoniGoManiHyperStrategy is initializing and by then the dataprovider is not yet available.

@rvasilev
Copy link
Contributor Author

rvasilev commented Jun 23, 2021

I see your point, but there's no way it can reduce the risk of leaking.

mgm-config-private.json should be out of git tree and placed in .gitignore by default if we're concerned about the leakage of sensitive data. Otherwize - pushing once to github will turn the catastrophe.

Obviously there should be mgm-config-private.example.json in tree.


Nothing is wrong with current config loading except that config should be overridable by freqtrade design. You can declare config in the strategy.py and override it in config.json. And after that - override more in config-1.json, config-2.json etc.

The main reason to override - there'll be no need to manually merge local changes and upstream changes to mgm-config after git pull. All locally tuned parameters could be stored in out of tree mgm-config-private.

@Rikj000
Copy link
Owner

Rikj000 commented Jun 23, 2021

Obviously there should be mgm-config-private.example.json in tree.

The one provided is an example, on my end I got that fixed up with Linux system links, but sure we can rename to .example and .gitignore + update the installation guide for it to protect contributing developers more.

The main reason to override - there'll be no need to manually merge local changes and upstream changes to mgm-config after git pull. All locally tuned parameters could be stored in out of tree mgm-config-private.

Same goes for this, we'll rename to .example and .gitignore + update the installation guide for it to improve upon the ease of git pulling.

I'll add following copy/paste command in the docs to make this really easy to do too:

cp ./user_data/mgm-config.example.json ./user_data/mgm-config.json && cp ./user_data/mgm-config-private.example.json ./user_data/mgm-config-private.json

@Rikj000 Rikj000 added Feature - Enhancement Update or improvement to existing feature Planned Planned feature, improvement or bugfix (not being worked on yet) and removed Question Further information is requested Won't Fix This will not be worked on labels Jun 23, 2021
@Rikj000 Rikj000 changed the title Overridable mgm-config with mgm-config-private / change MGM configuration loading logic Rename to .example.json config files and .gitignore original files Jun 23, 2021
@Rikj000 Rikj000 added this to the v0.13.0 milestone Jun 23, 2021
@rvasilev
Copy link
Contributor Author

OK. private parts will be protected from the accidental git-pushing.

If there's no other special reason to not merge mgm-config and mgm-config-private during strategy initialization and hyperopting, I'll PR in some time.

"exchange": ["name", "key", "secret"], "apiserver", "telegram"

will be popped out from config to not accidentally show on screenshots/hyperopt/backtest stdouts

@Rikj000
Copy link
Owner

Rikj000 commented Jun 23, 2021

If there's no other special reason to not merge mgm-config and mgm-config-private during strategy initialization and hyperopting, I'll PR in some time.

The fiat_display_currency I also have in the private section, since that way you don't have to share your local currency which gives away a part of where you are located when you share your mgm-config.json.

Same goes to bot_name, if people configure that, then it only personalizes the config file but changes nothing to the strategy, thus it makes shared files more cluttered and personalized.

But if you want feel free to PR the changes we discussed here! 🙂

@Rikj000 Rikj000 added In Progress This is being worked on and removed Planned Planned feature, improvement or bugfix (not being worked on yet) In Progress This is being worked on labels Jun 26, 2021
@Rikj000 Rikj000 closed this as completed Jun 28, 2021
@Rikj000 Rikj000 changed the title Rename to .example.json config files and .gitignore original files ⚡️ Rename to .example.json config files and .gitignore original files Sep 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Feature - Enhancement Update or improvement to existing feature
Projects
None yet
Development

No branches or pull requests

2 participants