abderrahim commented on code in PR #1879:
URL: https://github.com/apache/buildstream/pull/1879#discussion_r1389025273


##########
src/buildstream/_remotespec.py:
##########
@@ -253,6 +266,10 @@ def new_from_node(
         if auth_node:
             server_cert, client_key, client_cert = cls._parse_auth(auth_node, 
basedir)
 
+        keepalive_config = spec_node.get_mapping("keepalive-config", None)
+        if keepalive_config is not None:
+            keepalive_config.validate_keys(["keepalive-time"])

Review Comment:
   I think this is too much keepalive. We have only one keepalive setting, so 
no need to put it in a keepalive-config dictionary. We should either have it 
directly in the server config or (and I think this would be better) in a 
generic dictionary for connection settings that allows adding things later.



##########
src/buildstream/_protos/build/bazel/remote/asset/v1/remote_asset_pb2.py:
##########
@@ -4,9 +4,8 @@
 """Generated protocol buffer code."""
 from google.protobuf import descriptor as _descriptor
 from google.protobuf import descriptor_pool as _descriptor_pool
-from google.protobuf import message as _message

Review Comment:
   I think we were using a specific version of protobuf compiler to generate 
these so that they work with both old and new grpc.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to