-
-
Notifications
You must be signed in to change notification settings - Fork 403
#1447 - back-end usage scenario #1536
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add .proto file (messages and a service), generate go files. (Draft to demonstrate the proposal).
24f5c6b
to
4631037
Compare
Move to a proper location (fix compilation).
4631037
to
d13b45b
Compare
Simplify API (remove error)
9f90e2e
to
5e9e7e7
Compare
Add `FilesService` impl.
5e9e7e7
to
97a231f
Compare
|
||
message LoadFileRequest { | ||
// path as it's returned in LoadSketchResponse for instance | ||
string path = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could use repeated
to be able to return few file entires with just 1 call
// File content | ||
bytes content = 1; | ||
// is skipped in case of error | ||
ContentType type = 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be removed to avoid duplication. The reason it's still here is that the server might still return not the requested type (for any reason)
|
||
message LoadFileResponse { | ||
// File content | ||
bytes content = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or it could be smth like (depends on the design if whether we want to let the client know error code to add custom behaviour):
oneof {
bytes content = 1;
grpc.status error = 2;
}
Add TODO (Type support).
Interesting proposal but I have some concerns. Protocol buffers are not meant to be used with large data sets as per documentation, sending the whole content of a file via gRPC is not a great idea I think. But the major concern is about security, if the In the end I think this must not be a concern of the CLI, files exchange should be handled by another service. |
@silvanocerza Yeah, your concerns are more than reasonable. I've been also thinking about that. It seems if we limit the paths to If we're talking about sketches and compiled binaries - they don't exceed kilobytes (megabytes max), so size seems to be not a huge issue. Also we can compress (see I also agree that external (dedicated) service should do it better, but imo it's very small additional feature to provide a full set of needed functional so it seems to be not a big deal. I doubt that average user will be able to install smth like https://github.com/hubot-grpc/filesystem-grpc and want to deal with dockers, daemons, etc. But instead it will allow users to use their laptops/desktops or even Raspi's with arduino-cli to compile for Arduino from any mobile client (smartphones, tablet, smart devices, etc). |
This is a use case I was interested in, but I have opted for running CLI on a Pi, editing files over SSH and then running the appropriate command over SSH with the board attached to the Pi's USB port.
|
yes, i tried different scenarios including samba (and it worked). I'm thinking about "average user" and Arduino is awesome due to it's simplicity. So imo if it costs only 42 lines of code as in this MR lot's of people will say "thank you guys for doing our life easier" |
@silvanocerza will it work for you to address security concerns? i can update the MR |
@per1234 can you please close this? It's sitting in my list of open PRs that mention me since forever. 😅 Thanks! <3 |
Closing as stale |
Add .proto file (messages and a service), generate go files.
(Draft to demonstrate the proposal).
Please check if the PR fulfills these requirements
before creating one)
our contributing guidelines
UPGRADING.md
has been updated with a migration guide (for breaking changes)The MR Draft demonstrates the back-end scenario when client and server are on different hosts thus making direct files access problematic. So the client can't read neither the sketches content, nor the compiled binaries.
The feature is not implemented
There is "file system" service allowing to request file content.
titled accordingly?
This is an alternative to returning file content every here and there.
See how to contribute