Add support for BigTable Online Storage - #17
Conversation
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
b90ac39 to
8ed8d62
Compare
|
By the way, can we please do a comparison of the size of the serving jar before and after adding Bigtable? I remember some folks had reasonable concerns about bloat in both Core and Serving previously when we pulled in all the GCP dependencies. |
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
Signed-off-by: Oleksii Moskalenko <moskalenko.alexey@gmail.com>
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
45c1f6b to
71e74ca
Compare
| </dependency> | ||
|
|
||
|
|
||
| <dependency> |
There was a problem hiding this comment.
shouldn't this dependency be inside connector?
| } | ||
|
|
||
| // Same feature reference, but empty value | ||
| if (feastValue.equals(ValueProto.Value.ValCase.VAL_NOT_SET)) { |
There was a problem hiding this comment.
how actual value can be not set?
| ValueProto.Value feastValue = getFeatureValue(valueType); | ||
|
|
||
| // Same feature reference, but different type | ||
| if (valueType.equals(ValueProto.ValueType.Enum.INVALID)) { |
There was a problem hiding this comment.
how value type from spec can be invalid?
| return true; | ||
| } | ||
|
|
||
| return TYPE_TO_VAL_CASE.get(valueType).equals(feastValue.getValCase()); |
There was a problem hiding this comment.
we convert value from bigtable based on type from Spec. how they can not match?
|
|
||
| @BeforeAll | ||
| static void globalSetup() throws IOException { | ||
| coreClient = TestUtils.getApiClientForCore(FEAST_CORE_PORT); |
There was a problem hiding this comment.
this method has 200 loc. it's simply hard to read. can we split it into functions and maybe eliminate some common parts?
There was a problem hiding this comment.
And should we probably move data creation into relevant tests? instead of creating them all together.
There was a problem hiding this comment.
Yeap, it would be cleaner by refactoring common parts out. As for data creation, I think it's cleaner to isolate data generation before all tests start running and only keep retrieval logic in the tests.
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
3f8b8f4 to
b4c3a9f
Compare
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
9ea21c6 to
8ad0678
Compare
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
Signed-off-by: Oleksii Moskalenko <moskalenko.alexey@gmail.com>
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
c278b91 to
743816b
Compare
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
Before adding Bigtable: 89652017 bytes |
Signed-off-by: Terence Lim terencelimxp@gmail.com
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: