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 #417] gRPC design doc and protobuf models #594

Merged
merged 37 commits into from
Nov 22, 2021

Conversation

jinrongluo
Copy link
Contributor

Fixes ISSUE #417.

Motivation

  • use gRPC protocol to support polyglot eventmesh client SDK

Modifications

  • gRPC protocol design doc
  • gRPC protobuf model
  • gRPC protobuf code generation

Documentation

  • Does this pull request introduce a new feature? yes
  • If yes, how is the feature documented? yes

jinrongluo and others added 30 commits May 10, 2021 16:58
# Conflicts:
#	eventmesh-test/src/main/java/org/apache/eventmesh/http/demo/sub/service/SubService.java
@vongosling
Copy link
Member

vongosling commented Nov 17, 2021

You could refer here to exclude the grpc auto-created files from ck. https://docs.gradle.org/current/userguide/checkstyle_plugin.html#sec:checkstyle_tasks

@vongosling
Copy link
Member

What went wrong:
Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.
Execution failed for task ':checkLicense'.
Use '--warning-mode all' to show the individual deprecation warnings.

@qqeasonchen I think we have some improvement when resorting to GitHub's building checks.

@qqeasonchen
Copy link
Contributor

@jinrongluo if grpc implementation is a plugin, there may need a api for grpc/http/tcp etc.

@qqeasonchen
Copy link
Contributor

What went wrong: Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0. Execution failed for task ':checkLicense'. Use '--warning-mode all' to show the individual deprecation warnings.

@qqeasonchen I think we have some improvement when resorting to GitHub's building checks.

@keranbingaa can you help to fix it?

@vongosling
Copy link
Member

What went wrong: Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0. Execution failed for task ':checkLicense'. Use '--warning-mode all' to show the individual deprecation warnings.
@qqeasonchen I think we have some improvement when resorting to GitHub's building checks.

@keranbingaa can you help to fix it?

https://docs.gradle.org/7.3/userguide/compatibility.html

@vongosling vongosling added this to the 1.4.0 milestone Nov 17, 2021
@keranbingaa
Copy link
Contributor

@vongosling @qqeasonchen ok,I will try to fix it

@vongosling vongosling changed the base branch from develop to grpc November 18, 2021 01:33
@vongosling
Copy link
Member

vongosling commented Nov 18, 2021

@jinrongluo We could move code to grpc branch temporarily, review after merge @qqeasonchen But first now, we should resolve 2 failure checks.

@xwm1992
Copy link
Contributor

xwm1992 commented Nov 18, 2021

image
this is the reason for CI/CD check failed, please excute gradlew checkLicense task, check the licenses in the ..\incubator-eventmesh\build\reports\dependency-license\dependencies-without-allowed-license.json file, if theses licenses are allowed, please copy these texts into ..\tool\license\allowed-licenses.txt

@keranbingaa
Copy link
Contributor

image this is the reason for CI/CD check failed, please excute gradlew checkLicense task, check the licenses in the ..\incubator-eventmesh\build\reports\dependency-license\dependencies-without-allowed-license.json file, if theses licenses are allowed, please copy these texts into ..\tool\license\allowed-licenses.txt

@jinrongluo maybe you need do this to solve the failed check

@jinrongluo
Copy link
Contributor Author

image this is the reason for CI/CD check failed, please excute gradlew checkLicense task, check the licenses in the ..\incubator-eventmesh\build\reports\dependency-license\dependencies-without-allowed-license.json file, if theses licenses are allowed, please copy these texts into ..\tool\license\allowed-licenses.txt

@jinrongluo maybe you need do this to solve the failed check

Thanks @keranbingaa I adding the allowed licenses for grpc and protobuf.

the commit is here - 0ef7783

@qqeasonchen
Copy link
Contributor

@jinrongluo if grpc implementation is a plugin, there may need a api for grpc/http/tcp etc.

@jinrongluo we may discuss this first before merge.

@qqeasonchen
Copy link
Contributor

@jinrongluo if grpc implementation is a plugin, there may need a api for grpc/http/tcp etc.

@jinrongluo we may discuss this first before merge.

Glad to see the sdk-gprc implementation, runtime implementation will commit later, right?

@jinrongluo
Copy link
Contributor Author

@jinrongluo if grpc implementation is a plugin, there may need a api for grpc/http/tcp etc.

@jinrongluo we may discuss this first before merge.

Glad to see the sdk-gprc implementation, runtime implementation will commit later, right?

Yes, @qqeasonchen this PR is about protobuff message design. the actual implementation will be coming in next PR. Thanks.

@jinrongluo
Copy link
Contributor Author

@jinrongluo if grpc implementation is a plugin, there may need a api for grpc/http/tcp etc.

@jinrongluo we may discuss this first before merge.

I agree. in the actual grpc implementation, I will use plugins. This PR is only about design.

@qqeasonchen
Copy link
Contributor

@jinrongluo if grpc implementation is a plugin, there may need a api for grpc/http/tcp etc.

@jinrongluo we may discuss this first before merge.

I agree. in the actual grpc implementation, I will use plugins. This PR is only about design.

ok, then it can be merged first.

@qqeasonchen qqeasonchen merged commit dafa5a7 into apache:grpc Nov 22, 2021
@vongosling
Copy link
Member

It's important to note that this is quite a difficult refactoring. you could refer quarkus codes. https://quarkus.io/guides/grpc-getting-started.

I'd like to help to opt code for this feature, looking forwards to your code :-)

jinrongluo added a commit to jinrongluo/incubator-eventmesh that referenced this pull request Jan 11, 2022
* [Issue apache#337] Fix HttpSubscriber startup issue

* [Issue apache#337] test commit

* [Issue apache#337] revert test commit

* [Issue apache#337] Enhance Http Demo Subscriber by using ExecutorService, CountDownLatch and PreDestroy hook

* [Issue apache#337] Enhance Http Demo Subscriber by using ExecutorService, CountDownLatch and PreDestroy hook

* [Issue apache#337] Address code review comment for Subscriber Demo App

* adding license headers

* adding grpc build file

* [Issue#417] update settings.gradle

* [Issue#417] fix grpc generated code styles

* [Issue#417] fix grpc generated code styles

* [Issue#417] fix grpc generated code styles

* [Issue#417] fix grpc generated code styles and license issue

* [Issue#417] fix license issue

* [Issue apache#417] ignore checkstyle for generated files

* [Issue apache#417] adding allow licensed for grpc protobuf

Co-authored-by: j00441484 <[email protected]>
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.

5 participants