You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by GitBox <gi...@apache.org> on 2022/12/12 12:42:06 UTC
[GitHub] [incubator-pegasus] acelyc111 opened a new pull request, #1290: refactor(idl): unify the *.thrift files used by cpp and java-client
acelyc111 opened a new pull request, #1290:
URL: https://github.com/apache/incubator-pegasus/pull/1290
https://github.com/apache/incubator-pegasus/issues/1283
This patch including changes:
- move thrift file from `src/runtime/security/security.thrift` to `idl/security.thrift`
- remove `java-client/idl/*.thrift`
- add `struct request_meta` to *.thrift which is used by java-client only
- add java related namespace to *.thrift
- remove `java-client/idl/apache-licence-template` will isn't used now
- move `recompile_thrift.sh` from `java-client/idl` to `java-client/scripts`
- update some structures' and fields' name, there are differents of *.thrift files between `idl/*` and `java-client/idl/*` before
## Compatiblity changes for go client
| old name | new name |
| ---- | ---- |
| query_cfg_request | configuration_query_by_index_request |
| query_cfg_response | configuration_query_by_index_response |
| org.apache.pegasus.apps.meta.create_app_args | org.apache.pegasus.replication.admin_client.create_app_args |
| meta.create_app_result | org.apache.pegasus.replication.admin_client.create_app_result |
| org.apache.pegasus.apps.meta.drop_app_args | org.apache.pegasus.replication.admin_client.drop_app_args |
| meta.drop_app_result | org.apache.pegasus.replication.admin_client.drop_app_result |
--
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@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] Apache9 commented on pull request #1290: refactor(idl): unify the *.thrift files used by cpp and java-client
Posted by GitBox <gi...@apache.org>.
Apache9 commented on PR #1290:
URL: https://github.com/apache/incubator-pegasus/pull/1290#issuecomment-1352450863
OK, found this on stackoverflow
https://stackoverflow.com/questions/52882370/is-safe-to-rename-a-field-in-thrift-idl
SO thrift only relies on the field id, not the name.
Then I think it is less hurt for our users to upgrade.
--
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@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] codecov-commenter commented on pull request #1290: refactor(idl): unify the *.thrift files used by cpp and java-client
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #1290:
URL: https://github.com/apache/incubator-pegasus/pull/1290#issuecomment-1346457658
# [Codecov](https://codecov.io/gh/apache/incubator-pegasus/pull/1290?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> :exclamation: No coverage uploaded for pull request base (`master@c143575`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#section-missing-base-commit).
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #1290 +/- ##
=========================================
Coverage ? 53.40%
=========================================
Files ? 27
Lines ? 2522
Branches ? 0
=========================================
Hits ? 1347
Misses ? 1130
Partials ? 45
```
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] acelyc111 commented on pull request #1290: refactor(idl): unify the *.thrift files used by cpp and java-client
Posted by GitBox <gi...@apache.org>.
acelyc111 commented on PR #1290:
URL: https://github.com/apache/incubator-pegasus/pull/1290#issuecomment-1351833508
> I think we'd beter bump the major version to include this break changes?
>
> And do we have a compatibilty guide about what things can be broken while upgrading?
>
> And it is a bit strange that, why java client and cpp client can use different thrift defination while they connect to the same server?
1. That would be better, but I'm afraid there may be some more compatiblity changes in the future, the major version will inscrease too frequently then.
2. We will point that out obviously in the release note, as how we do before.
3. No matter what the name of the struct or field is, they are compatiable. When decode a request message from client on server, it depends on what the rpc code is, then use the related struct to decode it. For the fields, only the numberic tags are used, no matter what the names are.
--
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@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] acelyc111 merged pull request #1290: refactor(idl): unify the *.thrift files used by cpp and java-client
Posted by GitBox <gi...@apache.org>.
acelyc111 merged PR #1290:
URL: https://github.com/apache/incubator-pegasus/pull/1290
--
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@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org