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

Improvements and documentation for snapshotting #60

Merged
merged 7 commits into from
Apr 18, 2017

Conversation

johnbywater
Copy link
Collaborator

No description provided.

Changed snapshot class to use event entity version
integers (was timestamps). Added snapshotting to
example application. Added step about snapshotting
to usage section of README file.
Changed snapshot class to use event entity version
integers (was timestamps). Added snapshotting to
example application. Added step about snapshotting
to usage section of README file.
@coveralls
Copy link

coveralls commented Apr 18, 2017

Coverage Status

Coverage decreased (-0.3%) to 99.547% when pulling 4cbc084 on feature/documentation_for_snapshotting into 8f82a07 on develop.

@coveralls
Copy link

coveralls commented Apr 18, 2017

Coverage Status

Coverage decreased (-0.3%) to 99.547% when pulling 4cbc084 on feature/documentation_for_snapshotting into 8f82a07 on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 99.935% when pulling 7d867f8 on feature/documentation_for_snapshotting into 8f82a07 on develop.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 99.935% when pulling 7d867f8 on feature/documentation_for_snapshotting into 8f82a07 on develop.

@johnbywater johnbywater merged commit 44f82d3 into develop Apr 18, 2017
@johnbywater johnbywater deleted the feature/documentation_for_snapshotting branch April 18, 2017 23:57
last = page[-1]
page = list(query.filter(pk__token__gt=Token(last.pk)))

def delete_record(self, record):
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bit scary for event sourcing;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, it doesn't make something possible that was previously impossible. :-) I added this method to check the snapshots are working in all DBMSs. There is room for improvement in the snapshot tests, and maybe this could be removed. Or maybe it should be accompanied by a method to update an event? I'm not sure...

@yarbelk
Copy link

yarbelk commented Apr 19, 2017

Looks nice. One thing I was thinking of is the Cql models might do to have an ABC, and change any isinstance calls to check that. This makes it easier to set up different entities with different partion keys. I haven't looked much at the sql active records, so I don't know if a similar approach would make sense there.

Example - my use case has a fairly static, low entity count (maybe 20) aggregate, and then very dynamic, high count entity, which may be regionally partitioned.

Making an ABC would make sense both from an ease of implementation, and from an ease of upgrade.

@johnbywater
Copy link
Collaborator Author

Thanks. It seems simpler and better to make the snapshots have the version numbers of the events, rather than timestamps. Could possibly introduce a method to search for the last version at a given time.

I tried to make the active record classes derive from a common library supertype, but it broke the ORMs so I had to abandon the attempt - if that's what you mean?

Also, what did you mean by "high count entity, which may be regionally partitioned"?

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

Successfully merging this pull request may close these issues.

None yet

4 participants