https://pantsbuild.org/ logo
Join Slack
Powered by
Does anyone remember why `--remote-store-server` t...
# development
h
Does anyone remember why 
--remote-store-server
takes a list, rather than a single server like 
--remote-execution-server
? (Both Tom and I have been tripped up with 
--remote-store-server=localhost:xxx
appending, rather than overriding)
e
I think that used to populate a client maintained server set for load balancing. Perhaps a Scoot detail that leaked into the client.
h
That sounds plausible We're thinking we can address this gotcha by either 1) renaming to 
--remote-store-servers
2) converting to a single str both are churny, but better to do now before more people use remoting. and it tripped up both Tom and me for far longer than we'd like to admit
a
Convert to one :)
➕ 1
h
I have no idea how we deprecate something like that, but okay cool. Thanks!
Okay I don't think it's possible to deprecate 
--remote-store-server
to go from a list option to a single string through the options system.. Implicit adds are an issue, that 
store_server = "foo"
in 
pants.toml
means to add to the list, rather than override. This blocks converting your list syntax to a string syntax, there's no middle ground you can migrate to Only options I can think of: 1. Deprecate 
--remote-store-server
to be 
--remote-store-servers
, then deprecate that to be 
--remote-store-server
... 2. Break our deprecation policy. This is an Alpha API, but still, bleh
a
Can we detect when folks have >1 of them and issue a warning, and just make it a hard switch in type after a few releases?
h
Yes, but that wouldn't warn about:
Copy code
store_server = ["foo"]
(And another issue, we have zero idea if they're using that syntax or not. It's entirely abstracted by the options system)
a
Yeah, that's fair 😞
That said, it's a very easy fix for folks to make, so it doesn't feel like a terrible breaking change to make...
w
if you really want to do it, the rename wouldn’t be that onerous. would just go for it.
--remote-store-host
, etc
h
Bazel simply uses 
--remote-cache
, which we could translate to 
--remote-store
. But that's weird with 
--remote-execution
being a bool to disable RE mode
a
Let's resolve the "can we merge the flags" thread before we choose the naming?
👍 1
h
Agreed, but I think the answer is no to merging iiuc. Tom pointed out he needed separate config for BuildGrid to work
a
Cool 🙂
h
Related to this naming, Tom has proposed considering the user specifying the schema directly in the server value, i.e. 
http
vs 
https
. That's how Bazel does it This is what enables/disables TLS. I think that likely will scale better, as it allows the store server to work with http, https, grpc, grpcs, and unix sockets. We could add support for those if we want in the future
a
Sounds good 🙂
❤️ 1
f
do we want to call the schemes 
grpc
and 
grpcs
then? I was using Tonic’s naming convention of 
http
and 
https
for the scheme, but Tonic is a gRPC library so 
https
== 
grpcs
is implied by that fact
h
Oh interesting. Yeah, Bazel only uses 
{grpc,grpcs,unix}
for remote execution. But throws in 
http
and 
https
for caching To confirm, we are using grpc/grpcs for both services?
f
REAPI is a gRPC protocol
so yeah only grpc and grpcs matter
I guess Bazel has a HTTP but non-REAPI caching mode?
a
It does, REST-ish
I haven't seen 
grcp://
and 
grpcs//
used anywhere, and it's a little weird because you can speak gRPC over e.g. Unix sockets. On the other hand, apparently it's a scheme nginx uses: http://nginx.org/en/docs/http/ngx_http_grpc_module.html
So... Probably makes sense?
f
also is 
unix://
only supported for the proxy config? https://docs.bazel.build/versions/master/remote-caching.html#unix-sockets
bazel seems to still require you to use 
http://
etc. to configure the cache protocol
but 
unix://
is specified separately in a proxy option when using a proxy running on a UNIX socket
a
https://github.com/bazelbuild/bazel/pull/11722 added support for 
unix:
for remote cache/executor
f
ah so 
unix
really means 
grpc+unix
…
no need for us to solve this right now though since we don’t have UNIX socket support to my knowledge
👍 1
h
To summarize, sounds good for us to use 
grpc
and 
grpcs
for now?
👍 1
f
yup fine by me. bonus is less confusion for people coming from Bazel
👍 1
h
And perhaps we rename both 
--remote-store-server
and 
--remote-execution-server
, e.g. 
--remote-{store,execution}-host
? Naming suggestions? I do like server, but that deprecation is nigh impossible
f
“endpoint” ?
“host” ?
“nook”? because many people might read a book (or a cache) in a nook …
🙃 1
okay probably not that last one 🥁
a
--remote-store
and 
--remote-executor
?
I'm slightly wary of "host" because host is typically the part after the scheme, and we're proposing including the scheme
➕ 1
Endpoint or server work for me too
Or address, I guess
🤔 1
h
I think I like address, @fast-nail-55400 thoughts?
f
I don’t have a preference between 
endpoint
and 
address
👍 1
endpoint is what Tonic calls it but that’s an internal implementation detail
how much is 
address
closer to 
host
in meaning than 
endpoint
?
(re inclusion of scheme)
h
@witty-crayon-22786 any thoughts on 
--remote-execution-address
vs 
--remote-execution-endpoint
? I don't have much preference, but am about to start implementing this
w
nah, no preference
👍 1
(thanks for checking though)
❤️ 1
Open in Slack
PreviousNext
Powered by