feat(java datahub-client): add Java REST emitter #3781
Conversation
| @@ -22,33 +22,47 @@ | |||
| public class RESTEmitter { | |||
There was a problem hiding this comment.
Should we call this just "RestEmitter" ?
There was a problem hiding this comment.
Done. Please review.
| private static final int DEFAULT_CONNECT_TIMEOUT_SEC = 30; | ||
| private static final int DEFAULT_READ_TIMEOUT_SEC = 30; | ||
|
|
||
|
|
||
| @Getter | ||
| private final String gmsUrl; |
There was a problem hiding this comment.
What is the format of this URL? We should also rename to be "serverUrl" because we are trying to move away from calling the Metadata Service "gms"
| @@ -40,3 +40,4 @@ include 'docs-website' | |||
| include 'metadata-models-custom' | |||
| include 'entity-registry:custom-test-model' | |||
| include 'spark-lineage' | |||
| include 'metadata-integration:java:datahub-client' | |||
There was a problem hiding this comment.
Why is there 3 levels of package here? There should be just 2.
Alternatively we can just have a top-level "datahub-client" package
There was a problem hiding this comment.
Keeping it as is after considering slack discussion
|
|
||
| private void emit(List<MetadataChangeProposal> mcps) { | ||
| RESTEmitter emitter = emitter(); | ||
| RestEmitter emitter = emitter(); |
There was a problem hiding this comment.
We should ideally use a batch emit for this. @aseembansal-gogo is working on that API
There was a problem hiding this comment.
Please point me to the API documentation, will integrate it.
| return new RestEmitter(gmsUrl, DEFAULT_CONNECT_TIMEOUT_SEC, DEFAULT_READ_TIMEOUT_SEC, null); | ||
| } | ||
|
|
||
| public static RestEmitter create(String gmsUrl, int connectTimeoutSec, int readTimeoutSec) { |
There was a problem hiding this comment.
I'm assuming we have unit tests for this class (and McpEmitter?)
There was a problem hiding this comment.
We have unit tests at spark-lineage level which covers both of these classes.
| * A class that makes it easy to create new {@link MetadataChangeProposal} events | ||
| * @param <T> | ||
| */ | ||
| @JsonDeserialize(builder = MetadataChangeProposalWrapper.MetadataChangeProposalWrapperBuilder.class) |
There was a problem hiding this comment.
no longer needed as we are not using pure jackson to serialize anymore.
| T aspect; | ||
| String aspectName; | ||
|
|
||
| @JsonPOJOBuilder(withPrefix = "") |
There was a problem hiding this comment.
no longer needed as we're not using pure jackson to serialize anymore
Checklist