Edgewall Software
Modify

Opened 4 years ago

Closed 4 years ago

Last modified 21 months ago

#11581 closed enhancement (fixed)

Add an abstract base class for connection classes

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.1.2
Component: database backend Version:
Severity: normal Keywords:
Cc:
Release Notes:
API Changes:

Added an abstract base class ConnectionBase for database connection classes.

Description

Discussed in #11580, add an abstract base class for the connection classes: SQLiteConnection, PostgreSQLConnection and MySQLConnection. With this addition, implementations of any new database backends will have an interface to adhere to.

Attachments (0)

Change History (8)

comment:1 Changed 4 years ago by Ryan J Ollos

Milestone: 1.0.21.1.2
Status: newassigned

I guess these changes should probably go on the trunk.

comment:2 Changed 4 years ago by Ryan J Ollos

Initially I used the pre-Python26 pattern of defining a class with methods that raise NotImplementedErrors. However this doesn't prevent a user from instantiating the abstract base class (unless we also define an __init__ method that raises a NotImplementedError), and it doesn't perform any runtime checks when constructing the derived instance to guarantee that the derived class has implemented all of the abstract methods.

>>> import trac.db.api
>>> c = trac.db.api.Connection()
>>> c.like()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "trac/db/api.py", line 209, in like
    raise NotImplementedError
NotImplementedError

Using the Python 2.6 AbstractBaseClass module we can enforce that all of the abstractmethods have been implemented. If we didn't define like on the SQLiteConnection class, the result would be:

>>> import trac.db.sqlite_backend
>>> csqlite = trac.db.sqlite_backend.SQLiteConnection('/home/user/Workspace/tracdev/tracdev/db/trac.db')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: Can't instantiate abstract class SQLiteConnection with abstract methods like

Proposed changes can be found in log:rjollos.git:t11581. In particular, please let me know if you have any better text for the Connection class method docstrings.

comment:3 Changed 4 years ago by Ryan J Ollos

I decided to commit changes in #11600 and leave these changes for a few days to see if there is any feedback. Proposed changes have been rebased on the trunk in log:rjollos.git:t11581.3.

Last edited 4 years ago by Ryan J Ollos (previous) (diff)

comment:4 Changed 4 years ago by Ryan J Ollos

API Changes: modified (diff)
Resolution: fixed
Status: assignedclosed

Committed to trunk in [12762].

comment:5 Changed 4 years ago by Ryan J Ollos

I'm thinking that the abstract class Connection should be named ConnectionBase. ConnectionBase will provide better name differentiation from ConnectionWrapper.

Two existing base classes that follow this convention are WikiMacroBase and CachedPropertyBase. Several base classes such as Repository do not follow the convention. Most of the abstract classes can be seen in [12782].

Going forward, should we use the convention that base classes should end in Base? The detail could be added to TracDev/CodingStyle.

comment:6 Changed 4 years ago by Ryan J Ollos

Change from comment:5 committed in [12868].

comment:7 Changed 4 years ago by Ryan J Ollos

API Changes: modified (diff)

comment:8 in reply to:  5 Changed 4 years ago by Ryan J Ollos

Replying to rjollos:

Going forward, should we use the convention that base classes should end in Base? The detail could be added to TracDev/CodingStyle.

This detail was added to the coding style page in TracDev/CodingStyle@19.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.