Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ISSUE #1013] Java sdk split multiple sdk by communication protocol #4759

Closed
wants to merge 7 commits into from

Conversation

scwlkq
Copy link
Contributor

@scwlkq scwlkq commented Jan 28, 2024

Fixes #1013

Motivation

Java sdk split multiple sdk by communication protocol

Modifications

  1. I found that the EventMeshCommon class in the original TCP package was referenced by the classes of grpc, HTTP, and other modules, so I placed this class under the Common module and corrected the reference.This also eliminates the coupling between the modules of the Java SDK.
  2. I will split the original SDKs.
    image
    However, the Catalog requires the use of the selector and GRPC modules, so they are placed together. The same applies to the WorkFlow module and selector module.
    image
    image
  3. A small question: The build.gradle files in new module copyed from original Java-sdks. If they have duplicate dependencies, i will delete unnecessary dependencies.

I am not sure if this segmentation method is correct. If there are any shortcomings, please let me kown, and i will correct it.

Documentation

  • Does this pull request introduce a new feature? ( no)

Copy link

codecov bot commented Jan 28, 2024

Codecov Report

Attention: 173 lines in your changes are missing coverage. Please review.

Comparison is base (378aa75) 17.62% compared to head (5cdc582) 17.67%.

❗ Current head 5cdc582 differs from pull request most recent head bc6081a. Consider uploading reports for the commit bc6081a to get more accurate results

Files Patch % Lines
...sh/client/grpc/consumer/EventMeshGrpcConsumer.java 61.78% 53 Missing and 7 partials ⚠️
...entmesh/client/grpc/consumer/SubStreamHandler.java 57.97% 24 Missing and 5 partials ⚠️
...h/client/grpc/util/EventMeshCloudEventBuilder.java 79.38% 17 Missing and 10 partials ⚠️
...sh/client/grpc/producer/EventMeshGrpcProducer.java 54.05% 13 Missing and 4 partials ⚠️
...tmesh/client/grpc/producer/CloudEventProducer.java 66.66% 14 Missing ⚠️
...client/grpc/producer/EventMeshMessageProducer.java 74.41% 10 Missing and 1 partial ⚠️
...nt/grpc/exception/ProtocolNotSupportException.java 0.00% 10 Missing ⚠️
.../client/grpc/config/EventMeshGrpcClientConfig.java 66.66% 4 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #4759      +/-   ##
============================================
+ Coverage     17.62%   17.67%   +0.05%     
- Complexity     1784     1785       +1     
============================================
  Files           805      798       -7     
  Lines         29924    29838      -86     
  Branches       2580     2577       -3     
============================================
+ Hits           5273     5275       +2     
+ Misses        24169    24082      -87     
+ Partials        482      481       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Pil0tXia
Copy link
Member

I think the naming of eventmesh-sdk-java-tcp will be better than eventmesh-tcp-sdk-java. What do you think?

@scwlkq
Copy link
Contributor Author

scwlkq commented Jan 29, 2024

I think the naming of eventmesh-sdk-java-tcp will be better than eventmesh-tcp-sdk-java. What do you think?

ye,I think your naming method looks better.

@qqeasonchen
Copy link
Contributor

too much sdks for users, doesn't it?

@Pil0tXia
Copy link
Member

too much sdks for users, doesn't it?

makes sense.

cc @xwm1992

@xwm1992
Copy link
Contributor

xwm1992 commented Jan 29, 2024

Is it necessary to split java-sdk into so many modules?

@scwlkq
Copy link
Contributor Author

scwlkq commented Jan 29, 2024

The minimum granularity of the original Java-SDK was 'client':http-client、grpc-client、tcp-client、workflow-client、catalog-client,so I split it according to this dimension to let each SDK have a single responsibility as much as possible.

But it seems bring some inconvenience,such as users should dependent on multiple SDKs if users want to use the feature, bring more sdks, etc.

The issue originally aims to split java sdk to multiple sdk by communication protocol, implement the slimming of the java sdk.. Maybe another method to reduce SDKs' number and meet issue's requirement is: put http, tcp, grpc individually(split by protocol), put other clients in one SDK.

cn:
原来Java SDK的最小粒度是‘客户端’:http客户端、grpc客户端、tcp客户端、workflow客户端、catalog客户端,所以我按照这个维度进行分割使各个SDK之间尽可能单一职责。

但是这种分法带来了一些问题,比如用户如果想使用多个协议,需要依赖多个SDK 以及带来了很多SDK 等等。

这个Issue原来的目的是按照协议分割Java SDK使得Java SDK更加精简。也许另外一种减少SDK数量并且满足issue要求的方法是:将grpc、http、tcp独立开(按照协议),将其他的客户端放在一个SDK下。

@scwlkq
Copy link
Contributor Author

scwlkq commented Jan 29, 2024

Is it necessary to split java-sdk into so many modules?

Please provide some suggestions for splitting

@mxsm
Copy link
Member

mxsm commented Jan 30, 2024

My personal opinion is to divide them according to different languages, such as Rust, Java, and Go. Instead of dividing them for each protocol, dividing them too finely will lead to a very bloated project. At the same time, whether other language SDKs should also be divided in this way.

@Pil0tXia
Copy link
Member

What is the original intention of issue #1013?

@qqeasonchen
Copy link
Contributor

Is it necessary to split java-sdk into so many modules?

Please provide some suggestions for splitting

@scwlkq thank you for your contribution, but we think it is not the best time to do this refacoring now.

@scwlkq scwlkq closed this Feb 2, 2024
@scwlkq scwlkq changed the title [ISSUE 1013] Java sdk split multiple sdk by communication protocol [ISSUE #1013] Java sdk split multiple sdk by communication protocol Feb 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement] Java sdk split multiple sdk by communication protocol
5 participants