You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@buildstream.apache.org by "juergbi (via GitHub)" <gi...@apache.org> on 2024/05/31 14:50:51 UTC
[PR] Use buildbox-casd as remote asset proxy, if supported [buildstream]
juergbi opened a new pull request, #1922:
URL: https://github.com/apache/buildstream/pull/1922
buildbox-casd is already used as CAS proxy. Using buildbox-casd for all remote connections aims to improve robustness (e.g., consistent retry behavior) and will allow adding support for token-based authentication (#1913).
This does not yet use buildbox-casd as remote execution proxy
This depends on https://gitlab.com/BuildGrid/buildbox/buildbox/-/merge_requests/605 for the remote asset proxy to be used. However, BuildStream will fall back to direct gRPC connections if buildbox-casd doesn't support asset-only remotes.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use buildbox-casd as remote asset proxy, if supported [buildstream]
Posted by "juergbi (via GitHub)" <gi...@apache.org>.
juergbi commented on PR #1922:
URL: https://github.com/apache/buildstream/pull/1922#issuecomment-2161084004
> Is the intent there would be some kind of version check for when to enable this or what?
The branch includes a fallback path. I.e., with older buildbox-casd it will keep using direct connections to remote asset servers.
We may want to remove the fallback path and error out with older buildbox-casd versions in the future. I think we should wait with that at least until proxy support is also added for remote execution and #1913 is complete (and buildbox and buildbox-integration have been tagged with that support).
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use buildbox-casd as remote asset proxy, if supported [buildstream]
Posted by "abderrahim (via GitHub)" <gi...@apache.org>.
abderrahim commented on code in PR #1922:
URL: https://github.com/apache/buildstream/pull/1922#discussion_r1632942007
##########
src/buildstream/_cas/casremote.py:
##########
@@ -59,18 +59,7 @@ def _configure_protocols(self):
local_cas = self.cascache.get_local_cas()
request = local_cas_pb2.GetInstanceNameForRemotesRequest()
- cas_endpoint = request.content_addressable_storage
- cas_endpoint.url = self.spec.url
- if self.spec.instance_name:
- cas_endpoint.instance_name = self.spec.instance_name
- if self.spec.server_cert:
- cas_endpoint.server_cert = self.spec.server_cert
- if self.spec.client_key:
- cas_endpoint.client_key = self.spec.client_key
- if self.spec.client_cert:
- cas_endpoint.client_cert = self.spec.client_cert
- if self.spec.keepalive_time is not None:
- cas_endpoint.keepalive_time.FromSeconds(self.spec.keepalive_time)
+ request.content_addressable_storage.CopyFrom(self.spec.to_localcas_remote())
Review Comment:
I'm not sure what's the idiomatic thing to do with grpcio, but I feel it would be better to pass it like
```
self.spec.to_localcas_remote(request.content_addressable_storage)
```
Would that be possible? Is it something we'd like to do, or is the current way more idiomatic?
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use buildbox-casd as remote asset proxy, if supported [buildstream]
Posted by "juergbi (via GitHub)" <gi...@apache.org>.
juergbi commented on code in PR #1922:
URL: https://github.com/apache/buildstream/pull/1922#discussion_r1639371875
##########
src/buildstream/_cas/casremote.py:
##########
@@ -59,18 +59,7 @@ def _configure_protocols(self):
local_cas = self.cascache.get_local_cas()
request = local_cas_pb2.GetInstanceNameForRemotesRequest()
- cas_endpoint = request.content_addressable_storage
- cas_endpoint.url = self.spec.url
- if self.spec.instance_name:
- cas_endpoint.instance_name = self.spec.instance_name
- if self.spec.server_cert:
- cas_endpoint.server_cert = self.spec.server_cert
- if self.spec.client_key:
- cas_endpoint.client_key = self.spec.client_key
- if self.spec.client_cert:
- cas_endpoint.client_cert = self.spec.client_cert
- if self.spec.keepalive_time is not None:
- cas_endpoint.keepalive_time.FromSeconds(self.spec.keepalive_time)
+ request.content_addressable_storage.CopyFrom(self.spec.to_localcas_remote())
Review Comment:
I'm not sure either what's more idiomatic but your suggestion sounds reasonable to me and it avoids the need for `CopyFrom()`. I've updated the branch accordingly.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use buildbox-casd as remote asset proxy, if supported [buildstream]
Posted by "juergbi (via GitHub)" <gi...@apache.org>.
juergbi merged PR #1922:
URL: https://github.com/apache/buildstream/pull/1922
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use buildbox-casd as remote asset proxy, if supported [buildstream]
Posted by "juergbi (via GitHub)" <gi...@apache.org>.
juergbi commented on PR #1922:
URL: https://github.com/apache/buildstream/pull/1922#issuecomment-2154747491
https://gitlab.com/BuildGrid/buildbox/buildbox/-/merge_requests/605 is now merged.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use buildbox-casd as remote asset proxy, if supported [buildstream]
Posted by "nanonyme (via GitHub)" <gi...@apache.org>.
nanonyme commented on PR #1922:
URL: https://github.com/apache/buildstream/pull/1922#issuecomment-2160983374
Is the intent there would be some kind of version check for when to enable this or what?
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org