You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@eventmesh.apache.org by "Pil0tXia (via GitHub)" <gi...@apache.org> on 2024/04/14 07:53:23 UTC
[PR] [ISSUE #4834] Add protocol exclusive fields filter for `/v2/configuration` endpoint (eventmesh)
Pil0tXia opened a new pull request, #4835:
URL: https://github.com/apache/eventmesh/pull/4835
<!--
### Contribution Checklist
- Name the pull request in the form "[ISSUE #XXXX] Title of the pull request",
where *XXXX* should be replaced by the actual issue number.
Skip *[ISSUE #XXXX]* if there is no associated github issue for this pull request.
- Fill out the template below to describe the changes contributed by the pull request.
That will give reviewers the context they need to do the review.
- Each pull request should address only one issue.
Please do not mix up code from multiple issues.
- Each commit in the pull request should have a meaningful commit message.
- Once all items of the checklist are addressed, remove the above text and this checklist,
leaving only the filled out template below.
(The sections below can be removed for hotfixes of typos)
-->
<!--
(If this PR fixes a GitHub issue, please add `Fixes #<XXX>` or `Closes #<XXX>`.)
-->
Fixes #4834
### Motivation
Since `EventMeshTCPConfiguration`, `EventMeshHTTPConfiguration` and `EventMeshGrpcConfiguration` are subclasses of `CommonConfiguration`, their serialized JsonString would contain superclass fields, which enlarges the response body and brings in difficulty in getting `CommonConfiguration` fields.
### Modifications
- Add a PropertyFilter to remove superclass fields from the JsonString of `EventMeshTCPConfiguration`, `EventMeshHTTPConfiguration` and `EventMeshGrpcConfiguration`, then add `CommonConfiguration` into the response body.
- Add `configs` param which accepts two values, `exclusive` and `all`.
- Add unified exception handling
- The actual version number will be returned after merging https://github.com/apache/eventmesh/pull/4055.
### Documentation
- Does this pull request introduce a new feature? (yes)
- If yes, how is the feature documented? (JavaDocs)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
Re: [PR] [ISSUE #4834] Add protocol exclusive fields filter for `/v2/configuration` endpoint (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia merged PR #4835:
URL: https://github.com/apache/eventmesh/pull/4835
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
Re: [PR] [ISSUE #4834] Add protocol exclusive fields filter for `/v2/configuration` endpoint (eventmesh)
Posted by "xwm1992 (via GitHub)" <gi...@apache.org>.
xwm1992 commented on PR #4835:
URL: https://github.com/apache/eventmesh/pull/4835#issuecomment-2081290601
I don’t understand the purpose of your PR modification. Why do you want to filter out the fields of the parent class? The fields of the parent class should be some common fields, and the subclass is an extension. Why do you want to filter out the common fields?
@Pil0tXia
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #4834] Add protocol exclusive fields filter for `/v2/configuration` endpoint (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on PR #4835:
URL: https://github.com/apache/eventmesh/pull/4835#issuecomment-2081570966
The EventMesh Dashboard needs to retrieve the configuration of each EventMesh Runtime after it is connected to the cluster, store it in the database for display on the frontend pages.
Since `EventMeshTCPConfiguration`, `EventMeshHTTPConfiguration`, and `EventMeshGrpcConfiguration` are subclasses of `CommonConfiguration`, the current master branch's `/v2/configuration` admin endpoint will return redundant fields of `CommonConfiguration` in the configurations for TCP, HTTP, and GRPC. This leads to the following issues:
1. When displaying configurations on the frontend pages of EventMesh Dashboard, three copy of `CommonConfiguration` fields will be shown, wasting space and potentially confusing users.
2. EventMesh Dashboard needs to support maintaining multiple versions of EventMesh clusters. If EventMesh Dashboard removes duplicate fields, it needs to know which fields come from `CommonConfiguration`. If EventMesh Dashboard maintains a `CommonConfiguration` itself, but different versions of EventMesh Runtime have different `CommonConfiguration`, then EventMesh Dashboard will have multiple `CommonConfiguration` classes.
3. If EventMesh Runtime does not enable all three protocols TCP, HTTP, GRPC, and instead only enables two or one of them, EventMesh Dashboard needs to iterate over TCP, HTTP, GRPC to retrieve the values of this field when storing the fields of `CommonConfiguration` in the database.
These issues could have been avoided, eliminating the display of redundant content, avoiding the need to maintain the `CommonConfiguration` class on the EventMesh Dashboard side, and avoiding complex processing logic. This PR extracts the member fields of the `CommonConfiguration` class into the `commonConfiguration` JSON field to assist EventMesh Dashboard in distinguishing fields from `CommonConfiguration`.
Callers of the `/v2/configuration` admin endpoint can also specify the `configs` parameter as `all`, keeping the fields of `CommonConfiguration` in the configurations for TCP, HTTP, and GRPC.
In the future, we may consider integrating the metaStorage Plugin with `ConfigService` to report configurations to the registry. When we need this functionality, I can start working on this feature.
---
EventMesh Dashboard需要在接入集群后，获取每一台EventMesh Runtime的配置，存入数据库中，以供前端页面上展示。
由于`EventMeshTCPConfiguration`、`EventMeshHTTPConfiguration`、`EventMeshGrpcConfiguration`是`CommonConfiguration`的子类，因此，目前master分支中`/v2/configuration`管理端点将会在TCP,HTTP,GRPC的配置中都返回一份`CommonConfiguration`的成员字段，存在冗余。这会带来以下问题：
1. EventMesh Dashboard在前端页面上显示配置时，将会显示三份一模一样的`CommonConfiguration`，浪费空间，容易使用户迷惑。
2. EventMesh Dashboard需要支持同时维护多个版本的EventMesh集群。如果由EventMesh Dashboard来移除重复的字段，那么EventMesh Dashboard需要知道有哪些字段来自`CommonConfiguration`。如果EventMesh Dashboard自行维护一个`CommonConfiguration`，但是不同版本的EventMesh Runtime具有不同的`CommonConfiguration`，那么EventMesh Dashboard就会存在多个`CommonConfiguration`。
3. 如果EventMesh Runtime没有开启TCP,HTTP,GRPC全部三种协议，而是只开启了其中两种或一种，则EventMesh Dashboard在将`CommonConfiguration`的字段存入数据库时，需要依次遍历TCP,HTTP,GRPC，获取此字段的值。
这些问题本可以被避免，不用显示重复内容，不需要在EventMesh Dashboard侧维护`CommonConfiguration`类，不需要复杂的处理逻辑。本PR将`CommonConfiguration`类中的成员字段提取到`commonConfiguration`JSON字段下，以帮助EventMesh Dashboard分辨来自`CommonConfiguration`的字段。
`/v2/configuration`管理端点的调用者也可以指定`configs`参数为`all`，在返回结果中保留TCP,HTTP,GRPC三种协议的配置类中`CommonConfiguration`的字段。
在未来，我们可能会考虑将metaStorage Plugin与`ConfigService`结合，将配置上报到注册中心。当我们需要此功能时，我可以着手做这个特性。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org