GRPC is a highly descriptive spec, so I can’t see why we couldn’t effectively generate CLI to query the whole of the API.
Would that be good enough to scrap rest?
I guess we can also start with GRPC only, and then add the rest annotations later, if there is a need.
In terms of debugging that’s @calmh’s call - for me an integrated API CLI would be fine. As for dropping rest: That’s definitely not a short term option, only a mid- or even long-term one. We’d be killing an ecosystem of tools around Syncthing with that move (regardless of how nice and easy gRPC might be to implement, it’s still a major change that will take time).
I am talking about dropping rest for api v2. Api v1 is here to stay for some while.
1 Like
(If you drop the REST API, SyncTrayzor will die. I don’t have the time to rewrite that part of it I’m afraid.)
2 Likes
So it won’t be killed straight away as I said, and at the same time I think my C# is good enough to fix it up.
3 Likes
So yeah two things. There’s the current API - it’s not going away, for at least a few years. Definitely not while there are important integrations that depend on it. Nobody needs to worry about that.
But the current api is disorganized and has other warts. We’d like to refresh it. That refresh will be on a new /api/v2 url or something. The internal GUI will switch to it. We’ll document and recommend it. It might gain new features not present in the old, though it must be possible to still configure new features using the old config push method, etc.
I’m still quite adamant that the new api must also support a json mode.
3 Likes
So I’ve started chipping at the first hurdle of this, which is making all of our types that we’d expose via the API (namely config and all sub-elements of it) proto compatible.
Meaning the struct definition and what not need to be generated from proto, including all of the magic tags, special filesystem type enum values with specific naming etc.
gogoproto supports everything that we need out of the box (of what I’ve hit so far), but it would require excessive extension use, effectively requiring extension for each field to change field names from proto’s style guide lower snake case to our json api’s lower camel case, an additional extension for setting xml tag, and so on.
To remove the need for that, and to add better support for the things we need, gogoproto have what they call a “vanity” gogoproto plugin, whereby it’s a custom protoc plugin binary that you build, which gets the parsed proto as input, allows you to customizes any extensions you want (including gogoproto ones), and then requires you to invokes gogoproto.Main() which generates the actual code.
I am using this to set default options on all files (No Size, but have ProtoSize etc), adjust go package names, drive json and xml tag naming by default so you don’t have to specify them (yet which you can still override).
I am also adding our own extensions for things like restart:"true" and default:"3600", so we don’t have to use gogoprotos ugly:
[(gogoproto.more_tags = "xml:\"foo,attr\" default:\"bah\" restart:\"false\""];
and can just do
[(ext.default) = "bah", (ext.restart) = true];
XML and json stuff comes for free.
I think this is very flexible and neat, but it obviously comes with extra code to do that. I guess I am somewhat well versed in proto extensions as I (ab)use them at work, so this seems understandable and not scary to me, but I wonder how other maintainers feel of having effectively a custom proto generator and a proto file with custom extensions.
I think having this custom thing might also be helpful when generating json APIs or what not, as we could automatically add extensions to service definitions making them grpc-gateway compatible.
1 Like
So given I have not had any feedback, I am somewhat parking this, as I don’t feel like sinking a lot of time into something that will be later binned because it brings too much baggage.
This is a bit over my head: What you describe sounds neat. And if the custom generator plugin isn’t a complicated monster that only you can control, I don’t see why not. However there’s the “over my head” part: All my experience with this comes from Syncthing, and I didn’t venture deeply into proto/other serialization aspects. So a “looks fine” from me doesn’t add much confidence on top of you obviously considering it a good solution. Plus I haven’t seen any code and I was even unable to turn up the vanity plugin docs on a cursory search.
This is an example of the vanity API:
You build this binary, and tell protoc to use it, intead of gogoproto.
I was going to say that for the new API I don’t really care how we format the JSON so we might as well roll with the protobuf defaults. But of course we leak the config (and event) types to both versions of the API plus XML, so some compatibility is required there…
The vanity thing looks reasonable enough I guess (after looking at it for five seconds), we already have a somewhat complicated protobuf build step so we could have this .go file in scripts/ or whatever and build it as part of that.
(I’m not sure our default:"blah" tags are actually better than having each type just implement type Defaulter interface { Default() interface{} } to produce a default version of themselves, or having a hardcoded static default object in the code, or having the defaults defines as JSON or XML or other format directly. So that part can be worked around.)
I’m wondering what this means for GUIs like Syncthing Tray. I haven’t used protobuf or grpc so far. However, both seem to be libraries/generators written in C/C++ which shouldn’t be too hard to use, right? I’m only concerned about the “custom” part. I assume this means I wouldn’t only have to use the *.proto files from the Syncthing repository but also compile some Go code from there? Sounds like an interesting challenge to integrate that into my Git repository structure and the build system.
Custom part is only relevant for syncthing, as it just customizes go code generated, for the purpose of supporting xml serialization etc, not something that you might care about
1 Like