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

Billy enhancement proposal #24

Closed
fangpenlin opened this issue Aug 6, 2013 · 7 comments
Closed

Billy enhancement proposal #24

fangpenlin opened this issue Aug 6, 2013 · 7 comments

Comments

@fangpenlin
Copy link
Contributor

This project works fine, so far so good. However, the barrier for letting people getting work on this project is a little bit high, namely, technical debt should be reduced. I would like to do some major refactorying. Before we go into implementation, some goals to achieve are listed here to remind us.
#1. Documentation

We should write some documents. Developer could understand how to use this project, what are some classes and module for. Some comments in code could be helpful. For example, when you are using a dirty hack, it's better to write why you are doing this, and what kind of side effect it may have. Something might look like this

# NOTICE: this is a dirty hack to use foo module
# as xxx feature is not supported in foo version xx
# it's better to find a more clean way to do this
foo._dirty_hacky_call()

Also, when you left something unfinished or not sure what to do, you can leave a # TODO comment on it:

# TODO: get rid of the brute force and improve performance

High level document should also be added to make people understand the project easily. Such as some how-to-use-it API call example.

Here we demonstrate how to create a daily basis payout:

    api = billy.API(api_key='xxx')
    api.daily_payout(...)

Something like that.

Document is important, but do not overkill.
#2. Isolate data model from the table definition

Data model and business logic should be extracted from tables and put in some isolated classes. Do not expose too much SQLALchemy details to the client side (the web framework views). In this way, it is much easier to test, also, when we don't want to use another web framework, it is much easier to adopt. Or, when we are switching to a NO-SQL database, the data model in middle also do help. We don't need to rewrite the whole client-end code, the data model interface should remains almost the same, we only need to rewrite under layer database querying stuff. Some visual explanation would looks like this.

The original design

[SQLAlchemy tables with business logic] <-------- [Flask client-end code]

the new approach

[SQLAlchemy tables] <-------- [Business logic] <--------  [Flask client-end code]

#3. Use SQLIte as the default database storage

To make it as easier to test and deploy as possible, we should use SQLite as the default database storage. The way we use SQL should also be as standard as possible (do not use XX database only feature). In this way, developers who like to get involved can jump in and start hacking quickly without installing any database server on their machine. The testing suites can also be ran without any extra setups. Users can also even use SQLite as the database in production environment when it is really easy to get started.

When SQLite based operation is well tested and proved. Surely SQLAlchemy can support almost any other SQL database without too much effort.
#4. Define operations and usage scenarios clearly

I think it is necessary to define what kind of operation this service should provide and what kind of scenario they will be used. With those descriptions, we can then implement those feature correctly and understand when to use them. For example, Niko wants to charge protection fee weekly from Tony (ah.. what a badass). Then he can create a plan (or bill plan more clearly?) like this:

import billy

# create the goat plan to bill every 3 months.

billy.Plan.create(
    amount=100,
    description='weekly protection fee',
    trial_period=None,
    frequency=billy.Frequency.WEEKLY,
    interval=3
)

Otherwise, without clear blueprint in mind, people can hardly understand it.


Cannot recall some ideas passed my mind, will update this when they come back to me.

@pnegahdar
Copy link
Contributor

Hey

  1. Agreed. Documentation is sparse because everything is still being moved around heavily. Once we reach a slightly stable point a good day of pure documentation is needed

  2. This is a feature of SQLAlchemy's declarative models. We decided to go with declarative since it is more easily hackable since viewing that one class tells you all you need to know about the table structure and the logic behind it. It has its issues too like circular dependency issues and what not. Its worth investigating whether we should decouple these, but as of now its not too important.

  3. This I disagree with. Its recurring billy as a service. Making postgres a minimum dependency is not a much of a requirement. Keeping it in postgres gives us a lot of added power which after CB analysis doesn't seem to be worth switching. Also I don't want to give the end user the false assumption that its safe (or a good idea) to run it using sqllite in production...

  4. Maks sense.

@fangpenlin
Copy link
Contributor Author

@pnegahdar

Thanks for your input. About using SQLite, could you explain your concern? Because I am pretty sure SQLite can be used in production environment (although my main idea is to ease the barrier for developing billy). It's only a matter of what kind of scale it is, is it suitable. SQLite has its drawback, it is not good for highly concurrent read and write due to its locks.

SQlite is used in Android smart phones, iOS devices and many other production environments. OpenStack Swift (the open source project which provides Amazon S3 alike cloud service) also uses SQLite as the internal database storage of nodes in very large cluster without a problem (of course in production). You can also read this article

http://www.sqlite.org/testing.html

They have millions tests are running against SQLite, including all kind of crazy tests, that's very few open source projects have been done in this kind of quality assurance as far as I know.

As billy is a simple recurring payment system, I don't see there is any advance SQL feature would be required, otherwise, that might be a overkill. When the usage scenario is about thousands (or even million) recurring payment records, surely SQLite can do the job very well. It can also save some time of users to install and setup a real database before their volume hit the point for switching to a real database.

@pnegahdar
Copy link
Contributor

@victorlin

Thanks for that link, interesting testing scenarios there. My concern with SQLlite isnt the robustness of it, but rather the common-case usability of it. Here are some things that come to mind that pushed us towards Postgres:

  1. Balanced uses pg interally so it made sense since its already part of the stack
  2. the setup process for pg isn't much more difficult
  3. the deployment strategy will initially be heroku (an likewise I expect many devs to deploy to some sort of PaaS) which usually have ephemeral FS. This is often overlooked and can cause total loss in the event sqllite is used. Furthermore most PaaS seem to default to pg
  4. Metered and credit based billing ( Feature: Refill Billing #8 , Feature: Creditline billing #9 ) will likely make heavy uses of the enum type
  5. multiproc/threading web workers will cause issues (fsync) when we use gunicorn/uswgi workers/forking.

Currently we're not really limited to sqllite (Removing the constraints using pgsql_where and the enum types should make us fully compatible), so in the meanwhile I'm cool with it moving, but I predict in the long run we'll switch back and make use of the added power of pg. Do you have a pull I can review for sqllite compatibility?

@fangpenlin
Copy link
Contributor Author

@pnegahdar

A big point there is when you are going to run tests, or start some quick hacks, you will not like to install a SQL server just for it, unless it is a really interesting project. I had a completely start over branch there trying to reach best practices with minimum features, you can reference to it

https://github.com/victorlin/billy/tree/major-refactory

It uses SQLite as the backend. The way to run tests on my branch is just like this

python setup.py develop
python setup.py nosetests

And if there is a specific SQL server required, it would be

python setup.py
# 10~30 minutes for install the SQL server
python setup.py nosetests

I have two workplace, one PC at my home, one at my office with my Macbook Air. I just installed PostgresSQL on my home PC last night, for now, I am using Macbook, I will need to install it again to start developing this project. These time are all costs, bro.

Yes, I don't need to install the server every single time, but it is important to make it easy for people to get tests run and start hacking, otherwise, why do we open source it? Enum type is not a problem as SQLite can support it in a string manner.

Also, if you are running tests on a SQL server, then certainly you will need to be careful to maintain a clean environment for them. With SQLlite, you can always have a completely clean environment for tests everywhere.

I think we should see this in a team player role and open source developer perspective.

@pnegahdar
Copy link
Contributor

@victorlin as I said:

I'm cool with it moving.

I'm no longer full-time maintainer of billy, I will be working on it sometime this week to update the API but for other changes please check with @mahmoudimus and make the appropriate decision with a pull.

@mahmoudimus
Copy link
Contributor

@victorlin @pnegahdar you both have really valid arguments however I have to lean towards the side of @pnegahdar here.

@victorlin your concerns are easily addressed using something like Vagrant where we use VirtualBox to control the environment of hacking on Billy.

What you're really asking for is, how quickly can we get up and running to hack on Billy. Vagrant solves this problem. All you have to do is two steps:

  1. clone billy from git
  2. type vagrant up

And in 10 minutes, you have a virtualbox where you can start messing around w/ Billy.

The reason I think the SQLite is fallacious is - let's say you need another dependency, like redis or elasticsearch. Are you really going to limit yourself to lightweight libraries when most of the work can be done from off the shelf tools?

Let's do this the right way - this is a solved problem. Use Vagrant and move on.

@fangpenlin
Copy link
Contributor Author

Personally, I like to use tools that can let us reach the goals with minimum effort but also provide good quality. I also prefer lightweight weapons when heavy weapons cannot bring too much benefits. For this project billy, at this moment, at this complexity and requirements, there is no need for heavy weapon. The core business model here is pretty simple and clean, when to charge, who to charge, date-time calculations and so on, there is no need to use advance SQL features, at least for now I cannot see there is a need to.

However, yeah, you are right. There were no easy-to-use tools like vagrant for running stuff in VM. Now things changed. So, I will leave testing for core business logic on SQLite, and functional or integration tests running on PostgreSQL, as it would be our production environment. I will write a Vagrantfile there for setting up billy environment later.

And as I was MySQL user rather than PostgreSQL user, I am not familiar with advance features of it, if you see there are some advance features which help billy project a lot, please tell me, we may can use it. (however, that would possibly make billy bound to PostgreSQL, people can only use PostgreSQL afterward. So here is a trade off)

You know, experience can be right in this environment, but it would be wrong in another. Thanks for your input, what I did might be correct in previous context but may not be right here. Please let me know if you see anything I seeing it wrong in Balanced context.

@fangpenlin fangpenlin mentioned this issue Aug 19, 2013
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

No branches or pull requests

3 participants