Rudd-O / zfs-tools

ZFS synchronization and snapshotting tools
79 stars 41 forks source link

Refactor Class Structure for Dataset, Pool, Snapshot #34

Open JavaScriptDude opened 3 years ago

JavaScriptDude commented 3 years ago

With current class design, its not possible to distinguish objects using isinstance(obj, <type>) because Pool and Snapshot both inherit from Dataset.

Refactor the class layout to include an 'abstract' class ZfsItem(object) which Pool, Dataset and Snapshot would all inherit from. This would give you the ability to share properties / methods whilst allowing usage of strong type assertions of all the key Objects.

I have a hard fork of your code in my own library that I'm writing called [zfslib](https://github.com/JavaScriptDude/zfslib) that includes this change. I just made the tweak and so its not fully tested but basic smoke testing passed.

Rudd-O commented 3 years ago

Hm. I think I would like to use your library and delete the matching code on my project instead.

Rudd-O commented 3 years ago

Send me a ping to see how we can collab.

JavaScriptDude commented 3 years ago

I definitely welcome the collab opportunity. I sent you a message to your website Rudd-O.com regarding switching license for zfslib to Simplified BSD.

I have refactored quite a bit but we should be able to make your test suite work with my library. Will need to do some back porting to support older Python as I presently only target Python 3.7+

JavaScriptDude commented 3 years ago

I just added tooling tonight to allow for testing but have not written any unit tests on my end yet. I only have manual hand testing at this point but hope to start writing formal tests in the coming days.

JavaScriptDude commented 3 years ago

FYI - For test data, see PoolSet._load(_test_data:="") -or- Connection.load_poolset(..., _test_data="")

JavaScriptDude commented 3 years ago

FYI - Just did a manual integration of your test_sync.py script and got them to run with some bug fixes on my end and some minor tweaks in the test code. All tests passed.