-
Notifications
You must be signed in to change notification settings - Fork 22
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
add dirty rpc function definition for sync data #79
Conversation
:( |
@huan Could you please review this PR? |
Ok, I'll review it tonight. The reason that I have not reviewed it for the past two days is that I think there are two ways that we could implement this new feature: 1. Top-downFrom this PR I can see a design that we want to dirty a payload from top to down: the higher level puppet will call your 2. Bottom-upHowever, I'm thinking about doing this from down to top. It would be better for the lower level puppet to be in charge of managing the payload dirty updates. When a payload needs to be updated, the lower puppet will emit a ConclusionI did not think it very much yet so there might be some pros/cons between the top-down and bottom-up methods. Please feel free to let me know what you are thinking about the bottom-up ways to archive this new feature, and I'll try to compare them later tonight. |
My PR includes both If let me design the system, I would keep both
So I would prefer to implement both |
Thanks for your explanation, I'll investigate it more later tonight. P.S. If we can describe in detail about the design of this PR at the same time that it was created, we will be able to speed up the communication loop from the beginning. The puppet system is really important for the Wechaty ecosystem (which includes the GRPC design), that's why I'm always try my best to keep it as simple as possible. Appreciate your time and I'll get back to our conversion soon. |
@windmemory Could you please help me to list all the cases which require the users to have to call the I always keep thinking that whether we should treat the
|
For contact, we might modify the alias on phone, and we want to get that data synced in For room, members might leave, or change their room alias, to get the latest data, we need to sync the room data. The root cause for this design, is that not all changes of rooms or contacts are pushed to Do you have any better idea to solve this problem? |
@windmemory Thank you very much for those use cases, they are very persuasive. So let's keep your two-direction design for this version. After a good sleep, I believe we can reuse the event types (like Here's how: Top-downWe call Bottom-upWe emit a Other ChangesWe change all the All actions that want to dirty a payload need to call the new Both the ConclusionAfter this refactoring, we will:
Please feel free to let me know if you have any comments/suggestions/questions. |
@windmemory There is always a better way to make sure the CI will be happy: run tests locally! :) |
Local test shows different results from the CI build, not sure where went wrong, so I am fixing local test while pushing changes in parallel, I think this is the fastest way to fix the problem. |
Don't know how to fix the |
The code looks great now! It's very clean and beautiful, thank you very much for the time discussing with me, this improvement is very valuable! You can leave the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
🎉 |
I bumped the version for you. :) |
Link to wechaty/puppet-service#43 |
* fix: correct spell mistakes * 0.13.10 * This is a breaking change! * change python proto package name * update the version of chatie_grpc * try to install request (grpc/grpc-node#922 (comment)) * 0.15.2 * add grpc debug client link * update setup.py to fix pypi bug (wechaty#59) * update setup to fix pypi bug * change chatie-grpc generated dir * 0.15.2 * 0.15.3 (wechaty#60) * keep the package dir name in site-packages as `chatie_grpc` (wechaty#62) * 0.15.3 * keep package name as chatie_grpc not chatie-grpc * 0.15.4 (wechaty#63) * deploy release jar (wechaty#61) * fix maven,deploy jar to center oss * fix maven,deploy jar to center oss * add java to README.md * use better relative path for proto_path (zxh0/vscode-proto3#31 (comment)) * 0.15.5 * update pypi install_requires config (wechaty#64) * 0.15.4 * add install_requires * 0.15.5 * 0.15.6 * add NOTICE * 0.13.10 * 0.16.0 * 0.16.1 * Bump @chatie/tsconfig from 0.8.0 to 0.10.1 (wechaty#70) Bumps [@chatie/tsconfig](https://github.com/Chatie/tsconfig) from 0.8.0 to 0.10.1. - [Release notes](https://github.com/Chatie/tsconfig/releases) - [Commits](https://github.com/Chatie/tsconfig/commits) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * Bump grpc_tools_node_protoc_ts from 2.5.11 to 4.0.0 (wechaty#69) Bumps [grpc_tools_node_protoc_ts](https://github.com/agreatfool/grpc_tools_node_protoc_ts) from 2.5.11 to 4.0.0. - [Release notes](https://github.com/agreatfool/grpc_tools_node_protoc_ts/releases) - [Commits](agreatfool/grpc_tools_node_protoc_ts@v2.5.11...v4.0.0) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * Bump @chatie/git-scripts from 0.2.5 to 0.6.2 (wechaty#68) Bumps [@chatie/git-scripts](https://github.com/Chatie/git-scripts) from 0.2.5 to 0.6.2. - [Release notes](https://github.com/Chatie/git-scripts/releases) - [Commits](https://github.com/Chatie/git-scripts/commits) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * Update betterproto (wechaty#46) * specify python v3 * 0.16.2 * add php rpc (wechaty#76) * add php grpc * update usage * update usage * update usage * update * update usage * add php rpc * add PHP Grpc & maintainers * 0.16.3 * add all codeowners for languages * 0.16.4 * Bump @chatie/eslint-config from 0.8.1 to 0.12.1 (wechaty#75) Bumps [@chatie/eslint-config](https://github.com/Chatie/eslint-config) from 0.8.1 to 0.12.1. - [Release notes](https://github.com/Chatie/eslint-config/releases) - [Commits](https://github.com/Chatie/eslint-config/commits) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * fix eslint * 0.16.5 * add php action (wechaty#78) * add php action * add php action * add php action * add php action * add php action * add php action * add php action * add php action * add php action * add php action * add php action * add php action * tune layout for source code * Update README.md * v0.17 * fix to v0.17 * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * fix github url * 0.16.2 * 0.17.1 * add dirty rpc function definition for sync data (wechaty#79) * add dirty rpc function definition for sync data * try to fix tests * add rpc roomMemberPayloadDirty * add dirty event type * revise code according to comments * modify dirty methods and event * try to fix build * modify code according to review * fix build * hope this is the final build fix * this must be the final final build fix * fix proto naming convention * change property name according to comment * Update package.json * Update package.json Co-authored-by: Huan (李卓桓) <zixia@zixia.net> * add contact phone rpc call definition (wechaty#80) * add more methods related to new wechaty-puppept update (wechaty#81) * add more methods related to new wechaty-puppept update * fix test * fix typo and order * 0.17.3 * fix request structure according to comments * add phone to contact payload (wechaty#82) * bump version to publish new changes (wechaty#83) * add csharp nuget (wechaty#84) * proto add csharp namespace * add csharp solution * update * csharp add build powshell * Create csharp.yml * update csharp.yaml * update copy.ps1 * add test * test * update common.ps1 * test action args key * test powshell args * test github Secrets * update yaml * update nuget package as rohith/publish-nuget@v2 * update version info * github action add nuget VERSION_FILE_PATH * chsarp github action over * recovery github action * clear * update gitgnore * delete .vs folder * update README.md for csharp * README.md update csharp grpc doc * README.md add nuget introduction * add file stream and image stream rpc call (wechaty#88) * add file stream and image stream rpc call * fix test * 0.17.5 * add imageType to MessageImageStreamRequest * add @deprecated to MessageFile and MessageImage rpc * add change history in readme * 0.18.0 * Bump grpc_tools_node_protoc_ts from 4.1.5 to 5.0.0 (wechaty#87) Bumps [grpc_tools_node_protoc_ts](https://github.com/agreatfool/grpc_tools_node_protoc_ts) from 4.1.5 to 5.0.0. - [Release notes](https://github.com/agreatfool/grpc_tools_node_protoc_ts/releases) - [Commits](agreatfool/grpc_tools_node_protoc_ts@v4.1.5...v5.0.0) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * Bump ts-protoc-gen from 0.12.0 to 0.13.0 (wechaty#86) Bumps [ts-protoc-gen](https://github.com/improbable-eng/ts-protoc-gen) from 0.12.0 to 0.13.0. - [Release notes](https://github.com/improbable-eng/ts-protoc-gen/releases) - [Changelog](https://github.com/improbable-eng/ts-protoc-gen/blob/master/CHANGELOG.md) - [Commits](improbable-eng/ts-protoc-gen@0.12.0...0.13.0) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * Send message stream (wechaty#89) * add sendFileStream rpc call * add sendFileStream into puppet server impl * add readme * 0.18.1 * change filebox to data with bytes type * add name attribute into stream request and response * Fix go ci (wechaty#91) * Fix go ci * Update go.yml * add go_package * debug github action * debug github action * debug github action * keep the latest betterproto version (wechaty#92) * Use metadata (wechaty#93) * use metadata send extra info, remove extra attributes in stream message * 0.18.2 * use common message to send FileBox * rename FileBoxMessage to FileBoxChunk * use data instead of chunk * 0.18.3 * Update README.md * Improve stream message (wechaty#95) * improve stream message definition * 0.18.4 * change names * wip... * normalize name * 0.18.5 * normalize name * 0.18.6 * rename request & responses for more robust * clean name * 0.18.7 Co-authored-by: Huan LI (李卓桓) <zixia@zixia.net> * Export file box (wechaty#98) * export filebox from generated ts file * 0.18.8 * add some stream notices comments * 0.18.9 * fix file rename * 0.18.10 * link to wechaty#99 * 0.18.11 * fix go-grpc generate failed (wechaty#101) * 0.18.12 * fix .net grpc for the new file file_box.proto and update TargetFramework to netstandard2.0 (wechaty#104) * init openapi * Rename & Add OpenAPI Specification Generator (wechaty#107) * rename npm name to wechaty-grpc (wechaty#106) * add grpc http annotations (wip) * add grpc gateway image * add swagger info annotation * make payload dirty restful * fix name * add more restful endpoint definitions * fix streaming methods * better protoc tools installer script * clean openapi scripts * rename src/index -> src/mod * generate proto js files with annotation * 0.19.1 * add gRPC Web example * 0.19.2 * wechaty-grpc v0.20 * 0.20.1 * lint * check protoc version to make sure >= 3.5 (wechaty#116) * test ok * upgrade all deps * add more node version for testing * 0.20.2 * use @master version of github actions, enable cache * install 3rd party protos * 0.20.3 * fix npm publish * 0.20.4 * add openApi & proto for export (wechaty#119) * add openApi & proto for export * open-api -> openapi * 0.20.5 * Pypi rename wechaty grpc (wechaty#118) * follow latest grpc proto annotations & rename * deprecate merge-rpoto.sh (wechaty#46) * install third party proto files * 0.20.5 * install proto by github actions * 0.20.6 * add cache for pypi install * 0.20.7 * use abs path for install * 0.20.8 * explicit python3 & pip3 * 0.20.9 * 0.20.10 * 0.20.11 * sync swagger version to gRPC * 0.20.12 * compatible with grpc-dynamic-gateway (konsumer/grpc-dynamic-gateway#50) * cean * fix * 0.20.13 * use proto full path & fix openapi * 0.20.14 * fix python proto path * 0.20.15 * add openapi link * 0.20.16 * a workaround to fix issue wechaty#120 * clean * 0.20.17 * fix name * 0.20.18 * specify beta version for install * 0.20.19 Co-authored-by: Huan (李卓桓) <zixia@zixia.net> Co-authored-by: wj-Mcat <1435130236@qq.com> Co-authored-by: 犀利豆 <diaozxin@163.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> Co-authored-by: 一路向北 <zhangchunsheng423@gmail.com> Co-authored-by: Yuan Gao <wind.memory.cn@gmail.com> Co-authored-by: Darren <jesn2013@hotmail.com> Co-authored-by: 小雨 <614422099@QQ.com> Co-authored-by: Chao Fei <dchaofei@163.com>
* merge master 0.20.19 (#1) * fix: correct spell mistakes * 0.13.10 * This is a breaking change! * change python proto package name * update the version of chatie_grpc * try to install request (grpc/grpc-node#922 (comment)) * 0.15.2 * add grpc debug client link * update setup.py to fix pypi bug (#59) * update setup to fix pypi bug * change chatie-grpc generated dir * 0.15.2 * 0.15.3 (#60) * keep the package dir name in site-packages as `chatie_grpc` (#62) * 0.15.3 * keep package name as chatie_grpc not chatie-grpc * 0.15.4 (#63) * deploy release jar (#61) * fix maven,deploy jar to center oss * fix maven,deploy jar to center oss * add java to README.md * use better relative path for proto_path (zxh0/vscode-proto3#31 (comment)) * 0.15.5 * update pypi install_requires config (#64) * 0.15.4 * add install_requires * 0.15.5 * 0.15.6 * add NOTICE * 0.13.10 * 0.16.0 * 0.16.1 * Bump @chatie/tsconfig from 0.8.0 to 0.10.1 (#70) Bumps [@chatie/tsconfig](https://github.com/Chatie/tsconfig) from 0.8.0 to 0.10.1. - [Release notes](https://github.com/Chatie/tsconfig/releases) - [Commits](https://github.com/Chatie/tsconfig/commits) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * Bump grpc_tools_node_protoc_ts from 2.5.11 to 4.0.0 (#69) Bumps [grpc_tools_node_protoc_ts](https://github.com/agreatfool/grpc_tools_node_protoc_ts) from 2.5.11 to 4.0.0. - [Release notes](https://github.com/agreatfool/grpc_tools_node_protoc_ts/releases) - [Commits](agreatfool/grpc_tools_node_protoc_ts@v2.5.11...v4.0.0) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * Bump @chatie/git-scripts from 0.2.5 to 0.6.2 (#68) Bumps [@chatie/git-scripts](https://github.com/Chatie/git-scripts) from 0.2.5 to 0.6.2. - [Release notes](https://github.com/Chatie/git-scripts/releases) - [Commits](https://github.com/Chatie/git-scripts/commits) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * Update betterproto (#46) * specify python v3 * 0.16.2 * add php rpc (#76) * add php grpc * update usage * update usage * update usage * update * update usage * add php rpc * add PHP Grpc & maintainers * 0.16.3 * add all codeowners for languages * 0.16.4 * Bump @chatie/eslint-config from 0.8.1 to 0.12.1 (#75) Bumps [@chatie/eslint-config](https://github.com/Chatie/eslint-config) from 0.8.1 to 0.12.1. - [Release notes](https://github.com/Chatie/eslint-config/releases) - [Commits](https://github.com/Chatie/eslint-config/commits) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * fix eslint * 0.16.5 * add php action (#78) * add php action * add php action * add php action * add php action * add php action * add php action * add php action * add php action * add php action * add php action * add php action * add php action * tune layout for source code * Update README.md * v0.17 * fix to v0.17 * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * fix github url * 0.16.2 * 0.17.1 * add dirty rpc function definition for sync data (#79) * add dirty rpc function definition for sync data * try to fix tests * add rpc roomMemberPayloadDirty * add dirty event type * revise code according to comments * modify dirty methods and event * try to fix build * modify code according to review * fix build * hope this is the final build fix * this must be the final final build fix * fix proto naming convention * change property name according to comment * Update package.json * Update package.json Co-authored-by: Huan (李卓桓) <zixia@zixia.net> * add contact phone rpc call definition (#80) * add more methods related to new wechaty-puppept update (#81) * add more methods related to new wechaty-puppept update * fix test * fix typo and order * 0.17.3 * fix request structure according to comments * add phone to contact payload (#82) * bump version to publish new changes (#83) * add csharp nuget (#84) * proto add csharp namespace * add csharp solution * update * csharp add build powshell * Create csharp.yml * update csharp.yaml * update copy.ps1 * add test * test * update common.ps1 * test action args key * test powshell args * test github Secrets * update yaml * update nuget package as rohith/publish-nuget@v2 * update version info * github action add nuget VERSION_FILE_PATH * chsarp github action over * recovery github action * clear * update gitgnore * delete .vs folder * update README.md for csharp * README.md update csharp grpc doc * README.md add nuget introduction * add file stream and image stream rpc call (#88) * add file stream and image stream rpc call * fix test * 0.17.5 * add imageType to MessageImageStreamRequest * add @deprecated to MessageFile and MessageImage rpc * add change history in readme * 0.18.0 * Bump grpc_tools_node_protoc_ts from 4.1.5 to 5.0.0 (#87) Bumps [grpc_tools_node_protoc_ts](https://github.com/agreatfool/grpc_tools_node_protoc_ts) from 4.1.5 to 5.0.0. - [Release notes](https://github.com/agreatfool/grpc_tools_node_protoc_ts/releases) - [Commits](agreatfool/grpc_tools_node_protoc_ts@v4.1.5...v5.0.0) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * Bump ts-protoc-gen from 0.12.0 to 0.13.0 (#86) Bumps [ts-protoc-gen](https://github.com/improbable-eng/ts-protoc-gen) from 0.12.0 to 0.13.0. - [Release notes](https://github.com/improbable-eng/ts-protoc-gen/releases) - [Changelog](https://github.com/improbable-eng/ts-protoc-gen/blob/master/CHANGELOG.md) - [Commits](improbable-eng/ts-protoc-gen@0.12.0...0.13.0) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * Send message stream (#89) * add sendFileStream rpc call * add sendFileStream into puppet server impl * add readme * 0.18.1 * change filebox to data with bytes type * add name attribute into stream request and response * Fix go ci (#91) * Fix go ci * Update go.yml * add go_package * debug github action * debug github action * debug github action * keep the latest betterproto version (#92) * Use metadata (#93) * use metadata send extra info, remove extra attributes in stream message * 0.18.2 * use common message to send FileBox * rename FileBoxMessage to FileBoxChunk * use data instead of chunk * 0.18.3 * Update README.md * Improve stream message (#95) * improve stream message definition * 0.18.4 * change names * wip... * normalize name * 0.18.5 * normalize name * 0.18.6 * rename request & responses for more robust * clean name * 0.18.7 Co-authored-by: Huan LI (李卓桓) <zixia@zixia.net> * Export file box (#98) * export filebox from generated ts file * 0.18.8 * add some stream notices comments * 0.18.9 * fix file rename * 0.18.10 * link to #99 * 0.18.11 * fix go-grpc generate failed (#101) * 0.18.12 * fix .net grpc for the new file file_box.proto and update TargetFramework to netstandard2.0 (#104) * init openapi * Rename & Add OpenAPI Specification Generator (#107) * rename npm name to wechaty-grpc (#106) * add grpc http annotations (wip) * add grpc gateway image * add swagger info annotation * make payload dirty restful * fix name * add more restful endpoint definitions * fix streaming methods * better protoc tools installer script * clean openapi scripts * rename src/index -> src/mod * generate proto js files with annotation * 0.19.1 * add gRPC Web example * 0.19.2 * wechaty-grpc v0.20 * 0.20.1 * lint * check protoc version to make sure >= 3.5 (#116) * test ok * upgrade all deps * add more node version for testing * 0.20.2 * use @master version of github actions, enable cache * install 3rd party protos * 0.20.3 * fix npm publish * 0.20.4 * add openApi & proto for export (#119) * add openApi & proto for export * open-api -> openapi * 0.20.5 * Pypi rename wechaty grpc (#118) * follow latest grpc proto annotations & rename * deprecate merge-rpoto.sh (#46) * install third party proto files * 0.20.5 * install proto by github actions * 0.20.6 * add cache for pypi install * 0.20.7 * use abs path for install * 0.20.8 * explicit python3 & pip3 * 0.20.9 * 0.20.10 * 0.20.11 * sync swagger version to gRPC * 0.20.12 * compatible with grpc-dynamic-gateway (konsumer/grpc-dynamic-gateway#50) * cean * fix * 0.20.13 * use proto full path & fix openapi * 0.20.14 * fix python proto path * 0.20.15 * add openapi link * 0.20.16 * a workaround to fix issue #120 * clean * 0.20.17 * fix name * 0.20.18 * specify beta version for install * 0.20.19 Co-authored-by: Huan (李卓桓) <zixia@zixia.net> Co-authored-by: wj-Mcat <1435130236@qq.com> Co-authored-by: 犀利豆 <diaozxin@163.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> Co-authored-by: 一路向北 <zhangchunsheng423@gmail.com> Co-authored-by: Yuan Gao <wind.memory.cn@gmail.com> Co-authored-by: Darren <jesn2013@hotmail.com> Co-authored-by: 小雨 <614422099@QQ.com> Co-authored-by: Chao Fei <dchaofei@163.com> * feat: add options to FriendshipAddRequest * 0.20.20 * fix: bug * fix: add strong type FriendshipAddOptions for FriendshipAddRequest * fix: rename attr name * fix: add optional for FriendshipAddOptions * fix: use google.protobuf.StringValue for optional * fix: keep hello as string type, and rename inviter_contact_id and inviter_room_id * 0.21.2 Co-authored-by: Huan (李卓桓) <zixia@zixia.net> Co-authored-by: wj-Mcat <1435130236@qq.com> Co-authored-by: 犀利豆 <diaozxin@163.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> Co-authored-by: 一路向北 <zhangchunsheng423@gmail.com> Co-authored-by: Yuan Gao <wind.memory.cn@gmail.com> Co-authored-by: Darren <jesn2013@hotmail.com> Co-authored-by: 小雨 <614422099@QQ.com> Co-authored-by: Chao Fei <dchaofei@163.com>
Currently, we don't have a way of force the
wechaty-puppet-hostie
server to clear it's cache, so we can sync the data from the IM server. I would like to add several rpc calls in the proto, so we can use it to clear server side cache.