search

carnival-data / carnival

JVM property graph data unification framework
https://carnival-data.github.io/carnival/
GNU General Public License v3.0
7 stars 2 forks source link

Vine v2 #26

Closed augustearth closed 3 years ago

augustearth commented 3 years ago

Migrate v1 Vine/Method to the v2 JsonVine style. The two will be merged. The class JsonVine will be renamed to Vine as the code at that level is generic. At the VineMethod level, there will be JsonVineMethod and TabularVineMethod. This will be a multi-step migration. Should probably be a milestone rather than a single issue, but here we are.

augustearth commented 3 years ago

Not sure whether how best to go about building the new infrastructure in parallel with the old. Will start by removing extraneous references to carnival.core.vine package.

augustearth commented 3 years ago

After doing the above, it looks like references to carnival.core.vine are now limited to the carnival.core.vine package. So, we can probably just rename current carnival.core.vine to carnival.core.vineold and start fresh with an empty carnival.core.vine package. This is a breaking change of course, but we've already introduced breaking changes. So, why not.

augustearth commented 3 years ago

Next step is to prep the current code for the new TabularVine branch.

augustearth commented 3 years ago

There is definitely more to do. For one, there is a disparity between the cache methods of JsonVine and MappedDataTableVine. JV has a single cache file. MDTV has two. Created an object DataTableFiles to encapsulate the two files of a data table, but we should probably do something more standardized across vines. Will add an issue for this.

GenericDataTableVine* has to be added as well. Will create a separate issue. Hopefully, we can leverage inheritance and/or composition to share code between Mapped and Generic data table vines.

Also, it occurs to me that there are no facilities in the new vine classes to open/close database connections. Maybe that's ok. Will have to have a design discussion with @hjwilli. Things are probably at a good enough stopping point to merge with master.

augustearth commented 3 years ago

Furthermore, I do think it's time to make Vine v2 a milestone. Will close this issue with next commit, create a milestone, and add finer grained tickets to it.