api
api copied to clipboard
Change dr MaxConnection data type from int32 to uint32
-
[x] Change dr tcpSetting's MaxConnection int type from int32 to uint32
Refer to: https://istio.io/v1.5/docs/reference/config/networking/destination-rule/#ConnectionPoolSettings-TCPSettings
When configure a value to above red part value:
4294967295
, the value can't be parsed due to int32 can't meet it, so need to useint64
to meet it.
Hi @ningyougang. Thanks for your PR.
I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test
on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test
label.
I understand the commands that are listed here.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
We could not change the type of an API. The confusing docs actually means, if no value set, the value that istio applied to envoy is max uint32(2^32-1)
We could not change the type of an API. The confusing docs actually means, if no value set, the value that istio applied to envoy is max uint32(2^32-1)
Thanks for your reply.
Thanks for the PR!
I am not sure this is backwards compatible in protobuf? Envoy's is a uint32 so I am not sure using int64 here is appropriate as well
uint32 type is in [0 , 4294967295] , also meet the scope.
I already changed it to uint32
Consider the compatible issue
, unit32 is better.
Per proto, this does seem ok (https://developers.google.com/protocol-buffers/docs/proto3#updating). However, I am not sure I understand the motivation. Do you really need a number that int32 will not satisfy?
The motivation is the int32 is not enough for below value
so it is better to change it from int32 to uint32.
Not only maxConnection
field exist the problem, but also other fields as below also exist the same issue.
hm..it is better to change these fileds's data type from int32 to unit32 both.
btw, regarding Per proto, this does seem ok
You mean need some changes for some proto file? but i didn't find it.
@ningyougang: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
🚧 This issue or pull request has been closed due to not having had activity from an Istio team member since 2021-11-19. If you feel this issue or pull request deserves attention, please reopen the issue. Please see this wiki page for more information. Thank you for your contributions.
Created by the issue and PR lifecycle manager.