-
Notifications
You must be signed in to change notification settings - Fork 120
Offline buffering mechanism #185
base: master
Are you sure you want to change the base?
Conversation
bd623d3
to
5d4c63c
Compare
""" | ||
Abstract base class for all DCCs. | ||
""" | ||
__metaclass__ = ABCMeta | ||
|
||
@abstractmethod | ||
def __init__(self, comms): | ||
def __init__(self, comms, buffering_params): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be a check for buffering_params or it can be optional? Like if passed its okay otherwise it will be taken as None, as initialised automatically in init methods of different dccs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's fine as this is an abstract class
:param draining_frequency: frequency with which data will be published after internet connectivity established(like seconds). | ||
""" | ||
if not isinstance(table_name, basestring): | ||
log.error("Table name should be a string.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For log.error() or raise only abstract methods implemented in dcc.py like register are doing same. Should they also need to be changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine
@Venkat2811 Please review the code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
offline_database and offline_queuing classes have common functionalities like adding and draining to db/queue. These common features should be abstracted out into abstract class implementation and should have derived classes for specific implementation.
We should have a separate package under utilities to have all offline queuing related classes rather than having it under core.
if not isinstance(comms, DCCComms): | ||
log.error("DCCComms object is expected.") | ||
raise TypeError("DCCComms object is expected.") | ||
if not isinstance(draining_frequency, float) and not isinstance(draining_frequency, int): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Python has Number type. Also, combine draining_frequency check in this if condition check itself.
If the client faces network disconnectivity, published messages can be stored as a persistent or non-persistent storage.
Added queuing and database storage mechanism.
Added Readme file.