Add a MergeDeferredDataSource which combines multiple data sources #35
Conversation
elliottslaughter
force-pushed
the
merge-data-source
branch
from
cf6cbe5 to
46cb248
Compare
| tile_set, | ||
| field_schema, | ||
| } | ||
| } |
There was a problem hiding this comment.
Thoughts on basic unit tests for functions like this? They seem amenable to unit testing but maybe relying on the compiler for assurances is more common in Rust-land?
There was a problem hiding this comment.
Look at the most recent push and see if it seems reasonable.
In general, Rust users do rely a lot on the type system to make sure things work. And Rust's type system is pretty good, so I have a lot of confidence when things compile. But there are still classes of logic errors we hit, so it would be better to add more unit testing in general. So thanks for the reminder.
src/merge_data.rs
Outdated
| let EntryInfo::Slot { | ||
| short_name: slot0_short_name, | ||
| .. | ||
| } = &slots[0] | ||
| else { | ||
| panic!("unexpected variant result in slot 0"); | ||
| }; | ||
|
|
||
| let EntryInfo::Slot { | ||
| short_name: slot1_short_name, | ||
| .. | ||
| } = &slots[1] | ||
| else { | ||
| panic!("unexpected variant result in slot 1"); | ||
| }; | ||
|
|
||
| let EntryInfo::Slot { | ||
| short_name: slot2_short_name, | ||
| .. | ||
| } = &slots[2] | ||
| else { | ||
| panic!("unexpected variant result in slot 1"); | ||
| }; | ||
|
|
||
| assert_eq!(slot0_short_name, "S1"); | ||
| assert_eq!(slot1_short_name, "S2"); | ||
| assert_eq!(slot2_short_name, "S3"); |
There was a problem hiding this comment.
My only suggestion would be to keep related cases entirely together rather than group by "operation"
| let EntryInfo::Slot { | |
| short_name: slot0_short_name, | |
| .. | |
| } = &slots[0] | |
| else { | |
| panic!("unexpected variant result in slot 0"); | |
| }; | |
| let EntryInfo::Slot { | |
| short_name: slot1_short_name, | |
| .. | |
| } = &slots[1] | |
| else { | |
| panic!("unexpected variant result in slot 1"); | |
| }; | |
| let EntryInfo::Slot { | |
| short_name: slot2_short_name, | |
| .. | |
| } = &slots[2] | |
| else { | |
| panic!("unexpected variant result in slot 1"); | |
| }; | |
| assert_eq!(slot0_short_name, "S1"); | |
| assert_eq!(slot1_short_name, "S2"); | |
| assert_eq!(slot2_short_name, "S3"); | |
| let EntryInfo::Slot { | |
| short_name: slot0_short_name, | |
| .. | |
| } = &slots[0] | |
| else { | |
| panic!("unexpected variant result in slot 0"); | |
| }; | |
| assert_eq!(slot0_short_name, "S1"); | |
| let EntryInfo::Slot { | |
| short_name: slot1_short_name, | |
| .. | |
| } = &slots[1] | |
| else { | |
| panic!("unexpected variant result in slot 1"); | |
| }; | |
| assert_eq!(slot1_short_name, "S2"); | |
| let EntryInfo::Slot { | |
| short_name: slot2_short_name, | |
| .. | |
| } = &slots[2] | |
| else { | |
| panic!("unexpected variant result in slot 1"); | |
| }; | |
| assert_eq!(slot2_short_name, "S3"); |
There was a problem hiding this comment.
oh and that last case should read "unexpected variant result in slot 2" (up above as well)
This PR adds a new
MergeDeferredDataSourcethat combines multiple data sources into a single logical data source. There are a couple of use cases this addresses:--servethem on different nodes, and--attachthem all, stitching them back into a single logical profile.There's an accompanying backend change that goes along with this that I'm still working on. This change introduces the basic
MergeDeferredDataSourceas a building block that those changes can sit on top of.Known limitations:
--archivemode in the backend (i.e., you must use a live server). This could be supported, but it would be costly and I'm not honestly sure it's worth it. Alternatively we could handle this better in the backend.