Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR reverts a previously introduced protobuf field and its associated Go mappings for remote trigger capability configuration, while the team investigates an alternative approach using a feature flag.
Changes:
- Removed
registrationStatusUpdateTimeoutfromRemoteTriggerConfiginregistry.proto. - Updated capability registry encode/decode logic to stop reading/writing the removed field.
- Regenerated protobuf Go bindings and removed the corresponding Go config/defaults field.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/loop/internal/core/services/capability/capabilities_registry.go | Stops encoding/decoding the removed RemoteTriggerConfig field in gRPC responses/requests. |
| pkg/capabilities/pb/registry.proto | Removes the registrationStatusUpdateTimeout field from the protobuf schema. |
| pkg/capabilities/pb/registry.pb.go | Regenerated Go protobuf bindings reflecting the schema change. |
| pkg/capabilities/capabilities.go | Removes the config field and default/apply-defaults logic for the reverted setting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -14,7 +14,6 @@ message RemoteTriggerConfig { | |||
| google.protobuf.Duration messageExpiry = 4; | |||
| uint32 maxBatchSize = 5; | |||
| google.protobuf.Duration batchCollectionPeriod = 6; | |||
There was a problem hiding this comment.
registrationStatusUpdateTimeout (field number 7) was removed from RemoteTriggerConfig. To prevent accidental reuse of the old tag/name (which can cause wire-compatibility issues in protobuf), please add a reserved 7; and reserved "registrationStatusUpdateTimeout"; entry inside RemoteTriggerConfig.
| google.protobuf.Duration batchCollectionPeriod = 6; | |
| google.protobuf.Duration batchCollectionPeriod = 6; | |
| reserved 7; | |
| reserved "registrationStatusUpdateTimeout"; |
|
Are any other repos using this field? |
No, none. |
Reverting this change whist investigating alternative of using a feature setting flag. No proto serialisation issues to worry about here as no corresponding remote capability config changes have been in any env.