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

AccountStats Plugin #349

Merged
merged 11 commits into from
May 28, 2017
Merged

AccountStats Plugin #349

merged 11 commits into from
May 28, 2017

Conversation

rnevet
Copy link
Collaborator

@rnevet rnevet commented May 21, 2017

Description

Implemented a plugins structure, first plugin is AccountStats.
AccountStats fetches your loans history and provides a Daily Earnings Notification

TESTING STAGE

In progress, currently seeing mismatching results when compared to exported CSV data.

Types of changes

  • [N/A] Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • [N/A] 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.
  • [N/A] 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.

@rnevet
Copy link
Collaborator Author

rnevet commented May 22, 2017

Related to #240

@rnevet rnevet mentioned this pull request May 25, 2017
4 tasks
Copy link
Member

@Evanito Evanito left a comment

Choose a reason for hiding this comment

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

Robust, I like it!

~~~~~~~~~~~~~~~~~~~

The AccountStats plugin fetches all your loan history and provides statistics based on it.
Current implementation sends a earnings summery Notification (see Notifications sections) every 24hr.
Copy link
Member

Choose a reason for hiding this comment

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

Typo: Summary

Copy link
Collaborator Author

@rnevet rnevet May 26, 2017

Choose a reason for hiding this comment

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

My classic typo. Thanks!

def get_plugins_config():
active_plugins = []
if config.has_option("BOT", "plugins"):
active_plugins = config.get("BOT", "plugins").split(',')
Copy link
Member

Choose a reason for hiding this comment

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

Probably also want to strip values in case user puts spaces between plugin names.


# noinspection PyAttributeOutsideInit
def init_db(self):
self.db = sqlite3.connect(r'market_data\loan_history.sqlite3')
Copy link
Member

Choose a reason for hiding this comment

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

That r looks like a typo, can't find reference to it in the docs.

Unless it means "relative" to current directory?

Copy link
Collaborator Author

@rnevet rnevet May 26, 2017

Choose a reason for hiding this comment

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

it indicates a raw string - so \ don't need to be escaped. :)
https://docs.python.org/2.0/ref/strings.html

Copy link

Choose a reason for hiding this comment

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

You should use / as path separator (work on any system). Or os.path.join().

@rnevet
Copy link
Collaborator Author

rnevet commented May 26, 2017

@Evanito did you run the notification, was the total earnings calculation correct?

laxdog and others added 8 commits May 26, 2017 12:11
- Created Plugin system, allowing user to define which Plugins are loaded from config file
- Started implementation of AccountStats (fetching history)
Split Plugin events to before and after lending
Added notify event (every cycle at the moment)
@Evanito
Copy link
Member

Evanito commented May 26, 2017

It resorts to scientific notation if you only get a few satoshi in the past 24hrs, but otherwise worked like a charm.

I was unable to install hypothesis yet had no errors running.

@Evanito
Copy link
Member

Evanito commented May 26, 2017

Shouldn't you add the plugins = setting to the default config?

@rnevet
Copy link
Collaborator Author

rnevet commented May 26, 2017

I was thinking to stop adding optional setting to the example config, what do you think?

@rnevet rnevet merged commit bf54c3c into master May 28, 2017
@Reno007
Copy link

Reno007 commented May 28, 2017

Cool that this has now been merged! I have added the relevant line in the config file but am not getting the notification (have telegram enabled). Am I supposed to wait 24 hours? Anything else to enable?

@rnevet
Copy link
Collaborator Author

rnevet commented May 28, 2017

Place the plugins line under config [BOT], the implementation is spamming you'll notice it's working

@Evanito Evanito deleted the account_stats branch June 2, 2017 19:49
@tamaskan
Copy link

tamaskan commented Jun 3, 2017

Great, can't wait for the added functionality of #246 and #114 :-)

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.

6 participants