Connection singleton

[LaurentMontBlanc]

Motivation

We have a lot of different ways to access the database in our codebase. Although all are functioning it adds unnecessary complexity. We want to reduce the mental overhead when reading through the code. Therefor we want to replace all accesses to the database with a single approach. A singleton class was proposed for the time being.

TL;DR Create one instance to rule them all, one instance to find them, one instance to query them all, and databases bind them, in the land of planemint where the tokens lie

Implementation

We have a Singleton metaclass in planetmint/config.py. The Connection class should like the Config be derived from a Singleton. Instead of using planetmint/backend/connection.py::connect() to create database connections we want to use Connection() throughout the code. If Connection() is called the it should either return the Connection instance or if not existent create a new one and return it.

However, because we want to instantiate different subclasses of Connection the __call__() should look a little bit different. When creating the instance it should check the Config for the type of Connection it should instantiate.

class DBSingleton(type):
    _instances = {}

    def __call__(cls, *args, **kwargs):
        if cls not in cls._instances:
            cls._instances[cls] = TarantoolDBConnection | LocalMongoDBConnection # logic for choosing db connection should happen here
        return cls._instances[cls]

For more information on the singleton pattern take a look here

ToDos

• add a DBSingleton to planetmint/backend/connection.py
• planetmint/backend/connection.py::Connection must inherit DBSingleton
• remove connection from planetmint/lib.py::Planetmint.__init__() signature and replace calls to self.connection with Connection()
• replace imports of planetmint/backend/connection.py::connect with new planetmint/backend/connection.py::Connection
• adjust fixtures which use connect for test cases

[eckelj]

@LaurentDeMontBlanc I like the proposal! thanks for working this out!

[LaurentMontBlanc]

@roninx991 the goal of this issue is to replace all calls to a Connection instance with Connection().

Current State

At the moment the Planetmint instance stores the connection and passes it to the backend.query dispatcher.

# planetmint/lib.py
def __init__(self, connection=None):
  # ...
  # a Connection instance is stored in Planetmint.connection
  self.connection = connection if connection is not None else planetmint().backend.connect()

def get_transactions(self, txn_ids):
  # self.connection is passed to the dispatcher to decide which call should be used
  return backend.query.get_transactions(self.connection, txn_ids)

The fixtures use the connect() function to instantiate a database connection.

# tests/conftest.py
@pytest.fixture(scope="session")
def _setup_database(_configure_planetmint):
    from planetmint.config import Config

    print("Initializing test db")
    dbname = Config().get()["database"]["name"]
    conn = connect()

    _drop_db(conn, dbname)
    schema.init_database(conn, dbname)
    print("Finishing init database")

    yield

    print("Deleting `{}` database".format(dbname))
    conn = connect()
    _drop_db(conn, dbname)

    print("Finished deleting `{}`".format(dbname))

Proposed State

When Connection is handled as a singleton we no longer need to store the connection in the Planetmint instance. Instead we can just call Connection() wherever we need to pass it. This ensures that we use the same db connection everywhere which in turn increases maintainability for future refactors.

# planetmint/lib.py
def __init__(self):
  # self.connection is removed

def get_transactions(self, txn_ids):
  # Connection() is passed to the dispatcher to decide which call should be used
  return backend.query.get_transactions(Connection(), txn_ids)

This change also extends to the test cases and fixtures:

# tests/conftest.py
@pytest.fixture(scope="session")
def _setup_database(_configure_planetmint):
    from planetmint.config import Config

    print("Initializing test db")
    dbname = Config().get()["database"]["name"]

    _drop_db(Connection(), dbname)
    schema.init_database(Connection(), dbname)
    print("Finishing init database")

    yield

    print("Deleting `{}` database".format(dbname))
    _drop_db(Connection(), dbname)

    print("Finished deleting `{}`".format(dbname))

[roninx991]

Current State

Based on the above mentioned information and proposal, below is the current status:-

• DBSingleton class has been added to planetmint/backend/connection.py. This will be inherited by other classes to implement Singleton behaviour.
• Connection class has been added to planetmint/backend/connection.py. This class inherits the Singleton class which will be used interact with Tarantool or Mongo databases.
• DBConnection class has been added to planetmint/backend/connection.py. This is an interface/abstract class which will be inherited by TarantoolDBConnection and LocalMongoDBConnection classes.
• To use a backend connection (Tarantool or MongoDB), Connection() has to be called which will initialize the database connection based on the default config or kwargs passed to the constructor. To access the connection class, use Connection().conn.
• To change the connection using existing Connection() object, use Connection().connect(backend), where backend can be tarantool or localmongodb.

Remaining Issues

After these changes were made, some test cases failed which were fixed and the number of test cases are down to 2. These are :-

1. test_drop:- Tarantool Database spaces are not being dropped despite drop_database method working in other test cases. (planetmint/tests/backend/test_schema.py)
2. test_bigchain_class_initialization_with_parameters:- This test case was built such that Planetmint is initialized without connecting to database. However current code changes makes a connection to LocalMongoDB and this connection fails since the initializing params provided are dummy params.(planetmint/tests/test_core.py)

[LaurentMontBlanc]

@roninx991 the class diagram should look like this, where the DBSingleton is only responsible for instantiating or holding the LocalMongoDBConnection or TarantoolDBConnection as Connection. In this diagramm Connection can be seen as an abstract class that does not implement the specific methods but the derived classes are the implementing classes.

classDiagram
DBSingleton <|-- Connection
DBSingleton : __call__()

Connection <|-- TarantoolDBConnection
Connection <|-- LocalMongoDBConnection
Connection : run()
Connection : init_database()
Connection : drop_database()

TarantoolDBConnection : run()
TarantoolDBConnection : init_database()
TarantoolDBConnection : drop_database()

LocalMongoDBConnection : run()
LocalMongoDBConnection : init_database()
LocalMongoDBConnection : drop_database()

[eckelj]

locally, everything is working docker-compose up -d tarantool rm -rf ~/.tendermint && ./tendermint init && ./tendermint node --consensus.create_empty_blocks=false --rpc.laddr=tcp://0.0.0.0:26657 --proxy_app=tcp://localhost:26658 planetmint start pytest tests

at the same time: make run fails as planetmint cannot connect to the tarantool server/service. manually, this is working.

I don't have a clue why this happens. to be investigated