-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add flatbuffer tonic demo #3
Conversation
fb_tonic_demo/src/util/common.rs
Outdated
// items been encoded or decoded shall not have any non static references. | ||
// However flatbuffer related types always have a 'fbb lifetime bound, I found no way to implement | ||
// something like serde do. | ||
pub struct FlatBufferBytes { |
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.
这里没必要单独定义这么一个结构吧?直接用 Vec<u8>
是不是也够了?拿到字节流后,上层自己去做反序列化
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.
Make sense. Done.
format!("Service was not ready: {}", e.into()), | ||
) | ||
})?; | ||
let codec = crate::util::common::FlatBufferCodec::default(); |
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.
这里这样的话,不就写死了吗?按说应该根据 content type 来决定用哪个 codec
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.
目前根据tonic的build逻辑看,其实生成的service代码在接口定义的部分就hardcode了和encode相关Input和Output等类型了,例如(https://github.com/zealchen/serialization-benchmark-rs/blob/main/fb_tonic_demo/src/util/fb.helloworld.Greeter.rs#L212).
感觉tonic支持的custom codec (hyperium/tonic#999) 只是编译可配置,而不是运行时可配置。
不知道这个理解对不对?
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.
那这个我在确认下,目前这样应该也够用,但是觉得不太优雅。
.build(); | ||
|
||
tonic_build::manual::Builder::new() | ||
.out_dir("src/util") |
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.
这种构建生成的一般不放到 git 里面,一般都是 target 里面,我们之前生成 fb 代码是这么写的:
println!("cargo:rerun-if-changed=fbs/prom_write.fbs");
flatc_rust::run(flatc_rust::Args {
inputs: &[Path::new("fbs/prom_write.fbs")],
out_dir: Path::new("target/flatbuffers/"),
..Default::default()
})
.expect("flatc");
然后在 lib.rs 里这样写
#[path = "../target/flatbuffers/prom_write_generated.rs"]
mod prom_write;
pub use prom_write::*;
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.
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.
Best Practice. Done!
Rationale
Add flatbuffer tonic demo.
Detailed Changes
As for the core flatbuffer codec implementation in common.rs, there may be some better solutions to manage the flatbuffer objects. AFAIK, the associated type Encode / Decode of Trait Codec has a 'static lifetime bound which means items been encoded or decoded shall not have any non static references. However flatbuffer related types always have a 'fbb lifetime bound, which I found no way to implement something like what serde do.
Test Plan
Manual test.