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

Allow loan history DB name to be configured #588

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

Conversation

laxdog
Copy link
Collaborator

@laxdog laxdog commented Dec 5, 2017

Description

Allows multiple bots to run from the same directory, for example in a docker setup.

TESTING STAGE

Testing

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have read CONTRIBUTING.md
  • I fully understand Github Flow.
  • My code adheres to the code style of this project.
  • I have updated the documentation in /docs if I have changed the config, arguments, logic in how the bot works, or anything that understandably needs a documentation change.
  • I have updated the config file accordingly if my change requires a new configuration setting or changes an existing one.
  • I have tested the bot with no issues for 24 continuous hours. If issues were experienced, they have been patched and tested again.

@ghost ghost assigned laxdog Dec 5, 2017
@ghost ghost added the Testing Phase label Dec 5, 2017
@rnevet
Copy link
Collaborator

rnevet commented Dec 5, 2017

This doesn't cover all files used by the bot so it won't achieve the goal. For example the plugins files for AccountStats.

@laxdog
Copy link
Collaborator Author

laxdog commented Dec 5, 2017

I don't understand. Isn't the only file that's written to the loans_history one @rnevet ?

I can't see any other files in the AccountStats.py

@rnevet
Copy link
Collaborator

rnevet commented Dec 5, 2017

It is also used for charts plugin.
and what about the file for MarketAnalysis?

I'm not sure this approach is maintainable, maybe create a contained output folder?

@laxdog
Copy link
Collaborator Author

laxdog commented Dec 5, 2017

The market analysis data is actually best kept as the same file, that means every instance gets access to the data the others are writing. This is how I'm running it currently. That's why I added the exchange prefix to those files.

I can add the market_data folder as well if you want, but this particular file needs to be configurable too.

I see the reference in Charts.py, I'll make that change.

@rnevet
Copy link
Collaborator

rnevet commented Dec 5, 2017

I suggest we have shared_data , out instance_data folders... which will allow both the required behaviors..
How are you going to sync access the shared_data?

We should think that in the future, we (or other devs) might want to store data that is either shared or instance related... configuring the filename is tedious.

@laxdog
Copy link
Collaborator Author

laxdog commented Dec 5, 2017

I feel this is getting overly complicated for a simple change to allow a filename to be configurable. I'll try to get around to it. I'm still in the middle of the FlaskServer and FRR as min branches though.

The access is already synced if you're using the docker-compose file. It mounts the local dir as a volume and then links it into the working dir of the container so you have access to the market data folder.

@rnevet
Copy link
Collaborator

rnevet commented Dec 5, 2017

I agree, I thought it might be complicated.

@laxdog
Copy link
Collaborator Author

laxdog commented Dec 5, 2017

It's going to have to wait until after the flask stuff is finished. There will be too many conflicts that I don't want to have to resolve.

@laxdog laxdog added the On hold label Dec 5, 2017
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