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

Move storage format over to binary, this change allows the addition of: #12

Merged
merged 4 commits into from May 14, 2021

Conversation

xadaemon
Copy link
Contributor

Checksums, File format versioning and feature flags i.e: encryption, this change also makes the db files way more compact.

Other changes:
tighten except clauses in Util class, change some names to snake case

This is a more involved change so take the time to review it and consider it's impacts and ask away any doubts you may have about it

closes #10

Checksums, File format versioning and feature flags i.e: encryption, this change also makes the db files way more compact.

Other changes:
tighten except clauses in Util class, change some names to snake case
@xadaemon
Copy link
Contributor Author

PS: the assertion that if data is JSON serializable it can be stored to the file format remains valid

@saurabh0719 saurabh0719 self-requested a review May 13, 2021 14:20
@saurabh0719 saurabh0719 added the enhancement New feature or request label May 13, 2021
elara/elarautil.py Outdated Show resolved Hide resolved
@saurabh0719
Copy link
Owner

@DarthUdp My understanding is Elara can now store objects in the files (since you're moving it out of json) along with the checksum feature.

Can you possibly add some more comments and a few basic tests for this util function before I merge it in?

@saurabh0719
Copy link
Owner

You can add a new test file into the test folder. Merging these commits will essentially be a breaking change.

@xadaemon
Copy link
Contributor Author

No it cannot store arbitrary objects, such serialization is essentially not achievable without the use of the very unsafe pickle module, and even then it cannot store any and all objects, what an implementer of a class can do is implement the Serialize interface. But then they are just describing a way to encode their class state into primitive types that can then be serialized.

refs:

@saurabh0719
Copy link
Owner

No it cannot store arbitrary objects, such serialization is essentially not achievable without the use of the very unsafe pickle module, and even then it cannot store any and all objects, what an implementer of a class can do is implement the Serialize interface. But then they are just describing a way to encode their class state into primitive types that can then be serialized.

refs:

Yup, got it 👍🏽 thanks

@xadaemon
Copy link
Contributor Author

Do you think the code could use more comments?

@xadaemon
Copy link
Contributor Author

Do you think the code could use more comments?

To me the code is pretty self explanatory but since you will have to work on it eventually :)

@saurabh0719
Copy link
Owner

Do you think the code could use more comments?

To me the code is pretty self explanatory but since you will have to work on it eventually :)

Yup I think the code is fine. I'll format it later if needed. But please do add some basic tests.

@xadaemon
Copy link
Contributor Author

I added one test of a full cycle data store and recovery.

@saurabh0719
Copy link
Owner

I added one test of a full cycle data store and recovery.

Thanks, I'll merge the PR. :)

@saurabh0719 saurabh0719 merged commit 8536170 into saurabh0719:main May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add checksums to verify file integrity
2 participants