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

Deprecate clock_tick context manager #39

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
119 changes: 80 additions & 39 deletions temporal_sqlalchemy/bases.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,77 @@
import sqlalchemy as sa
import sqlalchemy.dialects.postgresql as sap
import sqlalchemy.orm as orm
import sqlalchemy.orm.base as base
import sqlalchemy.orm.attributes as attributes
import psycopg2.extras as psql_extras

from temporal_sqlalchemy import nine
from temporal_sqlalchemy.metadata import get_session_metadata

_ClockSet = collections.namedtuple('_ClockSet', ('effective', 'vclock'))
_PersistentClockPair = collections.namedtuple('_PersistentClockPairs',
('effective', 'vclock'))

T_PROPS = typing.TypeVar(
'T_PROP', orm.RelationshipProperty, orm.ColumnProperty)


class ActivityState:
def __set__(self, instance, value):
if not instance.temporal_options.activity_cls:
raise ValueError(
"Can't set activity state on instance of %r "
"because the activity class is None."
% type(instance).__name__)

# TODO should not be able to change activity once changes have
# TODO been made to temporal properties
setattr(instance, '__temporal_current_activity', value)

if value:
current_clock = instance.current_clock
current_clock.activity = value

def __get__(self, instance, owner):
if not instance:
return self

return getattr(instance, '__temporal_current_activity', None)

@staticmethod
def reset_activity(target, attr):
target.activity = None

@staticmethod
def activity_required(target, value, oldvalue, initiator):
if not target.activity and oldvalue is not base.NEVER_SET:
raise ValueError("activity required")


class ClockState:
def __set__(self, instance, value: 'EntityClock'):
setattr(instance, '__temporal_current_tick', value)
if value:
instance.clock.append(value)

def __get__(self, instance, owner):
if not instance:
return self

vclock = getattr(instance, 'vclock', 0) or 0 # start at 0 if None
if not getattr(instance, '__temporal_current_tick', None):
new_version = vclock + 1
instance.vclock = new_version
clock_tick = instance.temporal_options.clock_model(tick=new_version)
setattr(instance, '__temporal_current_tick', clock_tick)
instance.clock.append(clock_tick)

return getattr(instance, '__temporal_current_tick')
Copy link
Contributor

Choose a reason for hiding this comment

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

Same note here about using getattr without a default argument.

However, be warned: attributes starting with double underscores may not behave as you expect. Python has a concept of "private variables" that may result in an AttributeError getting thrown.

From the docs:

Any identifier of the form __spam (at least two leading underscores, at most one trailing underscore) is textually replaced with _classname__spam, where classname is the current class name with leading underscore(s) stripped. This mangling is done without regard to the syntactic position of the identifier, as long as it occurs within the definition of a class.

Because it's required to occur inside the class definition I think this will be okay, but I thought I'd point it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I think in this case it's okay, but it's a good call out


@staticmethod
def reset_tick(target, attr):
if target:
setattr(target, '__temporal_current_tick', None)


class EntityClock(object):
id = sa.Column(sap.UUID(as_uuid=True), default=uuid.uuid4, primary_key=True)
tick = sa.Column(sa.Integer, nullable=False)
Expand Down Expand Up @@ -50,11 +109,13 @@ def __init__(
temporal_props: typing.Iterable[T_PROPS],
clock_model: nine.Type[EntityClock],
activity_cls: nine.Type[TemporalActivityMixin] = None):

self.history_models = history_models
self.temporal_props = temporal_props

self.clock_model = clock_model
self.activity_cls = activity_cls
self.model = None

@property
def clock_table(self):
Expand All @@ -73,7 +134,7 @@ def history_tables(self):
@staticmethod
def make_clock(effective_lower: dt.datetime,
vclock_lower: int,
**kwargs) -> _ClockSet:
**kwargs) -> _PersistentClockPair:
"""construct a clock set tuple"""
effective_upper = kwargs.get('effective_upper', None)
vclock_upper = kwargs.get('vclock_upper', None)
Expand All @@ -82,25 +143,14 @@ def make_clock(effective_lower: dt.datetime,
effective_lower, effective_upper)
vclock = psql_extras.NumericRange(vclock_lower, vclock_upper)

return _ClockSet(effective, vclock)
return _PersistentClockPair(effective, vclock)

def record_history(self,
clocked: 'Clocked',
session: orm.Session,
timestamp: dt.datetime):
"""record all history for a given clocked object"""
state = attributes.instance_state(clocked)
vclock_history = attributes.get_history(clocked, 'vclock')
try:
new_tick = state.dict['vclock']
except KeyError:
# TODO understand why this is necessary
new_tick = getattr(clocked, 'vclock')

is_strict_mode = get_session_metadata(session).get('strict_mode', False)
is_vclock_unchanged = vclock_history.unchanged and new_tick == vclock_history.unchanged[0]

new_clock = self.make_clock(timestamp, new_tick)
attr = {'entity': clocked}

for prop, cls in self.history_models.items():
Expand All @@ -111,16 +161,14 @@ def record_history(self,

if isinstance(prop, orm.RelationshipProperty):
changes = attributes.get_history(
clocked, prop.key,
clocked,
prop.key,
passive=attributes.PASSIVE_NO_INITIALIZE)
else:
changes = attributes.get_history(clocked, prop.key)

if changes.added:
if is_strict_mode:
assert not is_vclock_unchanged, \
'flush() has triggered for a changed temporalized property outside of a clock tick'

new_clock = self.make_clock(timestamp, clocked.current_clock.tick)
# Cap previous history row if exists
if sa.inspect(clocked).identity is not None:
# but only if it already exists!!
Expand Down Expand Up @@ -184,6 +232,10 @@ class Clocked(object):
first_tick = None # type: EntityClock
latest_tick = None # type: EntityClock

# temporal descriptors
current_clock = None # type: ClockState
activity = None # type: typing.Optional[ActivityState]

@property
def date_created(self):
return self.first_tick.timestamp
Expand All @@ -194,22 +246,11 @@ def date_modified(self):

@contextlib.contextmanager
def clock_tick(self, activity: TemporalActivityMixin = None):
warnings.warn("clock_tick is going away in 0.5.0",
PendingDeprecationWarning)
"""Increments vclock by 1 with changes scoped to the session"""
if self.temporal_options.activity_cls is not None and activity is None:
raise ValueError("activity is missing on edit") from None

session = orm.object_session(self)
with session.no_autoflush:
yield self

if session.is_modified(self):
self.vclock += 1

new_clock_tick = self.temporal_options.clock_model(
entity=self, tick=self.vclock)
if activity is not None:
new_clock_tick.activity = activity

session.add(new_clock_tick)
warnings.warn("clock_tick is deprecated, assign an activity directly",
DeprecationWarning)
if self.temporal_options.activity_cls:
if not activity:
raise ValueError("activity is missing on edit") from None
self.activity = activity

yield self
17 changes: 9 additions & 8 deletions temporal_sqlalchemy/clock.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
from temporal_sqlalchemy import nine, util
from temporal_sqlalchemy.bases import (
T_PROPS,
ClockState,
ActivityState,
Clocked,
TemporalOption,
TemporalActivityMixin,
Expand Down Expand Up @@ -78,6 +80,10 @@ def temporal_map(*track, mapper: orm.Mapper, activity_class=None, schema=None):
backref_name = '%s_clock' % entity_table.name
clock_properties['activity'] = \
orm.relationship(lambda: activity_class, backref=backref_name)
cls.activity = ActivityState()
event.listen(cls, 'expire', ActivityState.reset_activity)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@NicoleZuckerman this the expire for the activity descriptor

for prop in tracked_props:
event.listen(prop, 'set', ActivityState.activity_required)

clock_model = build_clock_class(cls.__name__,
entity_table.metadata,
Expand All @@ -94,24 +100,19 @@ def temporal_map(*track, mapper: orm.Mapper, activity_class=None, schema=None):
clock_model=clock_model,
activity_cls=activity_class
)

cls.current_clock = ClockState()
event.listen(cls, 'expire', ClockState.reset_tick)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@NicoleZuckerman this is the expire for the current_clock descriptor

event.listen(cls, 'init', init_clock)


def init_clock(obj: Clocked, args, kwargs):
kwargs.setdefault('vclock', 1)
initial_tick = obj.temporal_options.clock_model(
tick=kwargs['vclock'],
entity=obj,
)
obj.current_clock = obj.temporal_options.clock_model(tick=kwargs['vclock'])

if obj.temporal_options.activity_cls and 'activity' not in kwargs:
raise ValueError(
"%r missing keyword argument: activity" % obj.__class__)

if 'activity' in kwargs:
initial_tick.activity = kwargs.pop('activity')

materialize_defaults(obj, kwargs)


Expand Down
24 changes: 0 additions & 24 deletions temporal_sqlalchemy/metadata.py

This file was deleted.

43 changes: 24 additions & 19 deletions temporal_sqlalchemy/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,25 @@

import sqlalchemy.event as event
import sqlalchemy.orm as orm
import sqlalchemy.util as util

from temporal_sqlalchemy.bases import TemporalOption, Clocked
from temporal_sqlalchemy.metadata import (
get_session_metadata,
set_session_metadata
)


def _temporal_models(session: orm.Session) -> typing.Iterable[Clocked]:
for obj in session:
TEMPORAL_METADATA_KEY = '__temporal'


def set_session_metadata(session: orm.Session, metadata: dict):
if isinstance(session, orm.Session):
session.info[TEMPORAL_METADATA_KEY] = metadata
elif isinstance(session, orm.sessionmaker):
session.configure(info={TEMPORAL_METADATA_KEY: metadata})
else:
raise ValueError('Invalid session')


def _temporal_models(iset: util.IdentitySet) -> typing.Iterable[Clocked]:
for obj in iset:
if isinstance(getattr(obj, 'temporal_options', None), TemporalOption):
yield obj

Expand All @@ -27,29 +36,25 @@ def persist_history(session: orm.Session, flush_context, instances):
obj.temporal_options.record_history(obj, session, correlate_timestamp)


def temporal_session(session: typing.Union[orm.Session, orm.sessionmaker], strict_mode=False) -> orm.Session:
def temporal_session(session: typing.Union[orm.Session, orm.sessionmaker],
**opt) -> orm.Session:
"""
Setup the session to track changes via temporal

:param session: SQLAlchemy ORM session to temporalize
:param strict_mode: if True, will raise exceptions when improper flush() calls are made (default is False)
:return: temporalized SQLALchemy ORM session
"""
temporal_metadata = {
'strict_mode': strict_mode
}

# defer listening to the flush hook until after we update the metadata
install_flush_hook = not is_temporal_session(session)
if is_temporal_session(session):
return session

opt.setdefault('ENABLED', True) # TODO make this significant
# update to the latest metadata
set_session_metadata(session, temporal_metadata)

if install_flush_hook:
event.listen(session, 'before_flush', persist_history)
set_session_metadata(session, opt)
event.listen(session, 'before_flush', persist_history)

return session


def is_temporal_session(session: orm.Session) -> bool:
return isinstance(session, orm.Session) and get_session_metadata(session) is not None
return isinstance(session, orm.Session) and \
TEMPORAL_METADATA_KEY in session.info
1 change: 0 additions & 1 deletion tests/test_concrete_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,6 @@ def test_doesnt_duplicate_unnecessary_history(self, session):
t.prop_a = 1
t.prop_c = datetime.datetime(2016, 5, 11,
tzinfo=datetime.timezone.utc)

session.commit()

assert t.vclock == 1
Expand Down
25 changes: 25 additions & 0 deletions tests/test_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,28 @@ def test_is_temporal_session_on_raw_session(self, session, connection):
assert not is_temporal_session(raw_session)
finally:
raw_session.close()

def test_different_sessions_update_vclock(self, session, connection, sessionmaker, newstylemodel):
session.add(newstylemodel)
assert newstylemodel.vclock == 1
session.commit()

# create different session
transaction = connection.begin()
second_session = sessionmaker(bind=connection)
refreshed_model = second_session.query(models.NewStyleModel).first()

# update row within new session
refreshed_model.activity = models.Activity(description="Activity Description")
refreshed_model.description = "a new str"
second_session.add(refreshed_model)
assert refreshed_model.vclock == 2
second_session.commit()

# see vclock is still 2 after second session commits
refreshed_model = second_session.query(models.NewStyleModel).filter_by(
id=newstylemodel.id).first()
assert refreshed_model.vclock == 2
# clear out db
transaction.rollback()
second_session.close()
10 changes: 5 additions & 5 deletions tests/test_temporal_model_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,11 +154,11 @@ def test_clock_tick_editing(self, session, newstylemodel):
session.commit()

activity = models.Activity(description="Activity Description #2")
with newstylemodel.clock_tick(activity=activity):
newstylemodel.description = "this is new"
newstylemodel.int_prop = 2
newstylemodel.bool_prop = False
newstylemodel.datetime_prop = datetime.datetime(2017, 2, 10)
newstylemodel.activity = activity
newstylemodel.description = "this is new"
newstylemodel.int_prop = 2
newstylemodel.bool_prop = False
newstylemodel.datetime_prop = datetime.datetime(2017, 2, 10)

session.commit()

Expand Down
Loading