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

refactor: refactor sdk layer #9

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kevinten10
Copy link

@kevinten10 kevinten10 commented Dec 28, 2021

Hi,这里是对原有runtime-sdk的改动(不包含reactor),我尽力保持所有类路径和使用方式的不变。

layer design

Here is the overall design of the modules:

        runtime-sdk
        ↑             ↑
    sdk-domain     spec-pb
        ↑                      
sdk-infrastructure    

runtime-sdk中拆分出了sdk-infrastructuresdk-domain模块。

sdk-infrastructure:

  • basic properties
  • basic exceptions
  • basic serializer
  • basic utils
  • basic values/enums

变更介绍

runtime-sdk中的基础定义类、工具类、属性类等下沉到sdk-infrastructure层,这将被多种上层实现复用。

非兼容的变更点

  • io.mosn.layotto.v1.domain.ApiProtocol移动到io.mosn.layotto.v1.value.ApiProtocol
    • 变更原因:domain路径将被sdk-domain包占用,使用value路径

sdk-domain:

  • API entities
  • Blocking Runtime API interface

变更介绍

将标准API的定义剥离到sdk-domain层,包括请求响应的实体类、接口定义。

独立的API定义模块sdk-domain,将有利于:

  • 上层实现的复用和拓展。
  • API定义的拓展,例如使sdk-domain模块实现dapr的API。
  • 逻辑的清晰,API定义只需follow社区标准,可由API维护人员进行更新;上层的java-sdk实现可以交给专业的java开发者。

注:sdk-domain仅包含同步调用的api,后续reactor模式的api将放在sdk-domain-rx模块中。

变更点

我没有改动原有的API接口和实体类的使用(仅在代码风格上做了统一,以及添加了toString这样的方法)。

不过我新增了configuration的API entities和API接口(接口方法先注释掉了,后续再添加实现)。

故和原有API兼容。

grpc

仍在保留在runtime-sdk中。

@kevinten10
Copy link
Author

@seeflood Hi,这里是基于 #7 中的讨论,对原有runtime-sdk的层级重构和代码格式优化。

理论上,这个PR是对使用方无感的,麻烦有空帮忙review一下~

@seeflood
Copy link
Member

感谢 我晚上看下!

@seeflood
Copy link
Member

seeflood commented Dec 28, 2021

感谢贡献,有几个问题需要讨论下~

关于sdk-infrastructure

  1. exceptions相关的类感觉适合放到domain包?
  2. 我看 sdk-domain 虽然依赖了 sdk-infrastructure ,但是好像domain没有import这里面任何代码?
  3. domain包和infrastructure的边界是啥,会让人有一定的理解成本,想再简单一点。为啥一定要把这里面代码单独抽出来一个包呀?
    拆这个包是为了让blocking sdk 和rx sdk复用代码么?

如果是为了代码复用,能否:

a. 简单一点,就把这些代码放在domain包里呢
(事实上,目前这俩包其实是当一个包用的,上层依赖了domain就依赖了这俩包)

        runtime-sdk
        ↑             ↑
    sdk-domain     spec-pb

b. 如果不行的话,改个名字呢,sdk-domain改叫sdk-api,spec-pb改名叫sdk-grpc-api,sdk-infrastructure改叫sdk-utils,

image

Copy link
Member

@seeflood seeflood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sdk-reactor包目前编译报错哈
image

@seeflood
Copy link
Member

seeflood commented Dec 28, 2021

(如果打字不好沟通的话,可以约个时间视频会议聊

@kevinten10
Copy link
Author

@seeflood good idea! 工作日下午怎么样?

@seeflood
Copy link
Member

@kevinten10 好呀 明天下午?你看几点合适

@kevinten10
Copy link
Author

要不就先暂定明天下午三点或者四点吧~

@seeflood
Copy link
Member

@seeflood seeflood self-assigned this Jan 5, 2022
@kevinten10
Copy link
Author

Iam busy at the end of the year.

Give me some time to finish the refactoring~

@seeflood
Copy link
Member

seeflood commented Mar 2, 2022

@kevinten10 hi, is this PR ready for review?
Besides, we have two refactor PR now, both #7 and this one. Should I close #7 ?

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.

2 participants