search

sparky8512 / starlink-grpc-tools

Random scripts and other bits for interacting with the SpaceX Starlink user terminal hardware
The Unlicense
482 stars 64 forks source link

Docker image missing generated gRPC protocol modules #74

Closed DurvalMenezes closed 1 year ago

DurvalMenezes commented 1 year ago

As per the subject: 

docker run --name='starlink-grpc-tools' ghcr.io/sparky8512/starlink-grpc-tools dump_dish_status.py

results in:

    This script requires the generated gRPC protocol modules. See README file for details.

Shouldn't the image come with these modules ready?

PS: the above happens exactly the same in both amd64 and aarch64 architectures.

sparky8512 commented 1 year ago

I don't include them in the Docker image for the same reason that I don't just include them with the source in this project: there is no clear license that allows for their redistribution. I may be being overly paranoid about this, as a few other GitHub projects have posted them, but I'd rather not press the issue (there's a little more discussion of my rationale for that in issue #66).

dump_dish_status.py is the only script that requires them, as I wanted to keep it as simple as possible, so did not include the use of yagrc for reflection. It's not really meant to be a useful script on its own, but an example for people who want to write their own scripts that use the grpc requests directly, instead of using the starlink_grpc module.

I could provide a script that generates the protocol modules and then runs that dump script, but that would require an additional pip package (grpcio-tools) and there are reasons for not wanting that in the Docker image, due to incompatibility with later versions of the protobuf package (last I checked). So I may want to do a separate Dockerfile for that. This is actually how the Dockerfile for this project used to work, before I added reflection support directly into starlink_grpc.

Rather than doing any of that, though, maybe it would be better to exclude the stuff that isn't relevant from the Docker image. Right now, it includes the entire source tree from this project, which has a lot of stuff that isn't useful for its purpose.

DurvalMenezes commented 1 year ago

Thanks for the detailed explanation.

maybe it would be better to exclude the stuff that isn't relevant from the Docker image.

after thinking about it a bit in the light of the explanation you just provided, I agree that's the best solution.

sparky8512 commented 1 year ago

The "latest" Docker image has been updated to a version that no longer has the script in question, so I'm closing this issue out.