feat: update sticky#38
Conversation
vahidlazio
force-pushed
the
update-sticky
branch
5 times, most recently
from
a9676ef to
9256078
Compare
vahidlazio
force-pushed
the
update-sticky
branch
from
9256078 to
56735bc
Compare
andreas-karlsson
left a comment
andreas-karlsson
left a comment
There was a problem hiding this comment.
I left a few comments, but didn't really check the materialization logic, it's too dense to follow, but I trust it's doing the right thing! 🙂
| use core::marker::PhantomData; | ||
| use fastmurmur3::murmur3_x64_128; | ||
| use std::collections::{HashMap, HashSet}; | ||
| use std::collections::{BTreeMap, HashMap, HashSet}; |
There was a problem hiding this comment.
Why a BTreeMap? Do we still have protos configured to use btrees?
There was a problem hiding this comment.
yeah .. seems like it.
|
|
||
| message MaterializationContext { | ||
| map<string, MaterializationInfo> unit_materialization_info = 1; | ||
| message MaterializationMap { |
There was a problem hiding this comment.
Why changing name to Map instead of Context?
There was a problem hiding this comment.
because it's a map and just thought to remove an unnecessary nested object , and that's how it works also in shadow resolver and flags resolver. so used the same type.
|
|
||
| // Context about the materialization required for the resolve | ||
| MaterializationContext materialization_context = 7; | ||
| map<string, MaterializationMap> materializations_per_unit = 2; |
There was a problem hiding this comment.
Isn't it a problem that we can't differentiate between an empty map and not set? I.e. how does the resolver know if we have fetched everything, but it was empty OR we haven't fetched?
There was a problem hiding this comment.
we will return the default object for materialization info, and set unit_in_info to false.
2 participants
No description provided.