refactor: generate static gRPC clients

[whynowy]

Explain what this PR does.

[vigith]

In Rust, we do not check in the gRPC generated files. Any reason for this?

[whynowy]

In Rust, we do not check in the gRPC generated files. Any reason for this?

[whynowy]

Even tonic check in generated files.

https://github.com/hyperium/tonic/tree/master/tonic-types/src/generated

[vigith]

Should we though? I am okay with checking in, but never felt there is any need 😀

[whynowy]

Should we though? I am okay with checking in, but never felt there is any need 😀

Otherwise we always need to keep a copy of the proto files in the sub directory, and get protoc in the build environment.

[vigith]

Otherwise we always need to keep a copy of the proto files in the sub directory, and get protoc in the build environment.

That is ugly, let's check it in.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.35%. Comparing base (3182db3) to head (ca621e7).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2128      +/-   ##
==========================================
+ Coverage   64.10%   64.35%   +0.24%     
==========================================
  Files         326      325       -1     
  Lines       31459    31449      -10     
==========================================
+ Hits        20166    20238      +72     
+ Misses      10255    10175      -80     
+ Partials     1038     1036       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[whynowy]

What is the reason removing protoc works in the CI, while removing it from the Dockerfile makes the local image build fail? I understand we still depend on numaflow-rs, which does not use generated code, thus protoc is needed, but why CI works?

@BulkBeing @yhl25 @vigith

[BulkBeing]

It is because of the Github build caching.
We use below action to setup Rust:

- name: Setup Rust toolchain
  uses: actions-rust-lang/[email protected]
  with:
    cache-workspaces: rust -> target
    rustflags: ""

As part of its post run hooks, the target/ directory is cached (and restored on next run).

If I run the build on a fresh ubuntu:24.04 image on my laptop, I get:

root@0152f1cd39ee:/app/rust# RUSTFLAGS='-C target-feature=+crt-static' cargo build --target aarch64-unknown-linux-gnu
   Compiling aws-lc-rs v1.9.0
   Compiling aws-lc-sys v0.21.2
   Compiling rustls v0.23.13
   Compiling tonic v0.12.3
   Compiling numaflow v0.1.1 (https://github.com/numaproj/numaflow-rs.git?rev=30d8ce1972fd3f0c0b8059fee209516afeef0088#30d8ce19)
   Compiling toml v0.8.19
   Compiling rustix v0.38.37
   Compiling prometheus-client v0.22.3
   Compiling nkeys v0.4.4
   Compiling nom v7.1.3
   Compiling num-bigint v0.4.6
   Compiling rcgen v0.13.1
error: failed to run custom build command for `numaflow v0.1.1 (https://github.com/numaproj/numaflow-rs.git?rev=30d8ce1972fd3f0c0b8059fee209516afeef0088#30d8ce19)`

Caused by:
  process didn't exit successfully: `/app/rust/target/debug/build/numaflow-dfa21ca6daaf72dd/build-script-build` (exit status: 101)
  --- stdout
  cargo::rerun-if-changed=proto/source.proto
  cargo::rerun-if-changed=proto/sourcetransform.proto
  cargo::rerun-if-changed=proto/map.proto
  cargo::rerun-if-changed=proto/reduce.proto
  cargo::rerun-if-changed=proto/sink.proto
  cargo::rerun-if-changed=proto/sideinput.proto
  cargo::rerun-if-changed=proto/batchmap.proto
  cargo::rerun-if-changed=proto

  --- stderr
  thread 'main' panicked at /root/.cargo/git/checkouts/numaflow-rs-05379ba45fd0abbd/30d8ce1/build.rs:16:29:
  failed to compile the proto, Custom { kind: NotFound, error: "Could not find `protoc`. If `protoc` is installed, try setting the `PROTOC` environment variable to the path of the `protoc` binary. To install it on Debian, run `apt-get install protobuf-compiler`. It is also available at https://github.com/protocolbuffers/protobuf/releases  For more information: https://docs.rs/prost-build/#sourcing-protoc" }
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
warning: build failed, waiting for other jobs to finish...

Then I tried installing protobuf-compiler and the build was successful. Now if I uninstall protobuf-compiler and re-run the build, it still works:

root@0152f1cd39ee:/app/rust# apt remove -y protobuf-compiler
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
The following packages were automatically installed and are no longer required:
  libprotobuf-dev libprotobuf-lite32t64 libprotobuf32t64 libprotoc32t64 zlib1g-dev
Use 'apt autoremove' to remove them.
The following packages will be REMOVED:
  protobuf-compiler
0 upgraded, 0 newly installed, 1 to remove and 0 not upgraded.
After this operation, 164 kB disk space will be freed.
(Reading database ... 14753 files and directories currently installed.)
Removing protobuf-compiler (3.21.12-8.2build1) ...

root@0152f1cd39ee:/app/rust# apt autoremove
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
The following packages will be REMOVED:
  libprotobuf-dev libprotobuf-lite32t64 libprotobuf32t64 libprotoc32t64 zlib1g-dev
0 upgraded, 0 newly installed, 5 to remove and 0 not upgraded.
After this operation, 20.2 MB disk space will be freed.
Do you want to continue? [Y/n] Y
(Reading database ... 14717 files and directories currently installed.)
Removing libprotobuf-dev:arm64 (3.21.12-8.2build1) ...
Removing libprotobuf-lite32t64:arm64 (3.21.12-8.2build1) ...
Removing libprotoc32t64:arm64 (3.21.12-8.2build1) ...
Removing libprotobuf32t64:arm64 (3.21.12-8.2build1) ...
Removing zlib1g-dev:arm64 (1:1.3.dfsg-3.1ubuntu2.1) ...
Processing triggers for libc-bin (2.39-0ubuntu8.3) ...

root@0152f1cd39ee:/app/rust# protoc --version
bash: /usr/bin/protoc: No such file or directory

root@0152f1cd39ee:/app/rust# RUSTFLAGS='-C target-feature=+crt-static' cargo build --target aarch64-unknown-linux-gnu
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.18s

Looks like protoc is only invoked if the .proto files are changed. I tried adding a new line in source.proto, the build started failing with the first error.

[whynowy]

Ha, so we can not remove protoc dep until we have the generated code checked in in numaflow-rs :)

[yhl25]

LGTM!

[BulkBeing]

I think we should add ignore annotation for clippy too.

[whynowy]

#[allow(clippy::all)] - Is this right? @BulkBeing

[vigith]

perhaps #![warn(clippy::style)] ?

[BulkBeing]

I think it should be #[allow(clippy::all)]. The #![warn(clippy::style)] is enabling style warnings for the whole crate. I tried running clippy on numaflow-grpc, there are no warnings. The openapi generated code has a lot of warnings.

[BulkBeing]

The generated code already has the necessary annotations. So we don't have to do anything.

/// Generated client implementations.
pub mod map_stream_client {
    #![allow(
        unused_variables,
        dead_code,
        missing_docs,
        clippy::wildcard_imports,
        clippy::let_unit_value,
    )]
    use tonic::codegen::*;