Issue 209: Update Schema Registry to consume latest Pravega Client #221
Conversation
Signed-off-by: Shashwat Sharma <[email protected]>
Signed-off-by: Shashwat Sharma <[email protected]>
Signed-off-by: Shashwat Sharma <[email protected]>
Signed-off-by: Shashwat Sharma <[email protected]>
shshashwat
requested review from
kotlasaicharanreddy,
shrids,
sarlaccpit and
medvedevigorek
and removed request for
kotlasaicharanreddy and
shrids
Signed-off-by: Shashwat Sharma <[email protected]>
Signed-off-by: Shashwat Sharma <[email protected]>
Signed-off-by: Shashwat Sharma <[email protected]>
Signed-off-by: Shashwat Sharma <[email protected]>
There was a problem hiding this comment.
I don't understand where you are using the Pravega Client to access these tables. All I see is that this code sends wire commands directly to the segment store which is the wrong way of accessing Key-Value Tables.
All Key-Value Tables must be accessed via the APIs in io.pravega.client.tables. Do not send wire commands directly to the segment store using your own code.
So as per discussion, I'm removing that test which would be included back with the changes suggested as part of another PR |
Signed-off-by: Shashwat Sharma <[email protected]>
…om/shshashwat/schema-registry into issue209-use_latest_pravega_client
Signed-off-by: Shashwat Sharma <[email protected]>
shshashwat
requested review from
andreipaduroiu and
pbelgundi
and removed request for
sarlaccpit
test/src/test/java/io/pravega/schemaregistry/integrationtest/TestPravegaClientEndToEnd.java
Show resolved
Hide resolved
There was a problem hiding this comment.
The upgrade itself seems ok, but I have some comments:
- Why are we doing the same as Pravega's Controller SegmentHelper class in this repo? Shouldn't we be able to import SegmentHelper class here and stop duplicating code that induces more maintenance costs?
- Also, Schema Registry is a separate service from Pravega. As it uses KVT as a client, why it is sending low level Wire Commands? @andreipaduroiu's suggestion of using the KVT Client (as any other application) looks as the right approach.
- There is a test that has been removed as part of this change, I don't know why.
server/src/main/java/io/pravega/schemaregistry/storage/client/TableStore.java
Outdated
Show resolved
Hide resolved
server/src/main/java/io/pravega/schemaregistry/storage/client/WireCommandClient.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Approve, assuming that issue #222 is addressed after this one (including the deleted test).
Signed-off-by: Shashwat Sharma <[email protected]>
shrids
changed the title
Change log description
In latest Pravega Client few API's have been changed which needs to be handled in Schema Registry
Purpose of the change
Fixes #209
What the code does
Consumes the latest Pravega Client(0.10) by updating the Client API's
How to verify it
Import Pravega Version should be 0.10 (Snapshot) and all existing test cases should pass with a green build scan