initial attempt to support rama in shuttle #1943
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Initial integration of the Rama framework into Shuttle, providing support for both transport-layer (TCP) and application-layer (HTTP) services using Rama's alpha release 0.2.0-alpha.4.
- Method name typo in
RamaService::trasnport()needs to be corrected totransport()inservices/shuttle-rama/README.md - Missing error handling for
.awaitcall inservices/shuttle-rama/src/lib.rsline 103 could lead to potential hangs - Documentation should clarify TLS termination capabilities and limitations within Shuttle's environment
💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!
5 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile
There was a problem hiding this comment.
PR Summary
(updates since last review)
This update adds implementation details for the Rama service integration, including service trait implementations and type wrappers for both transport and application layers.
- Added
RamaServicewrapper struct inservices/shuttle-rama/src/lib.rswith state management and service implementations - Implemented
shuttle_runtime::Servicetrait for both transport and application layer services - Added type aliases
ShuttleRamaTransportandShuttleRamaApplicationfor cleaner return types
2 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
| stream | ||
| .write_all(resp.as_bytes()) | ||
| .await | ||
| .expect("write to stream"); | ||
|
|
There was a problem hiding this comment.
logic: Using expect() here could cause panic in production. Consider proper error handling and propagation.
There was a problem hiding this comment.
PR Summary
(updates since last review)
This update adds example code and documentation for the Rama framework integration, demonstrating both Application and Transport service implementations.
- Added comprehensive examples in
services/shuttle-rama/README.mdshowing both HTTP and TCP service patterns - Updated version reference to Rama 0.2.0-alpha.5 in documentation and dependencies
- Missing GitHub example link in
codegen/src/lib.rsservice documentation table
3 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
| @@ -24,6 +24,7 @@ mod shuttle_main; | |||
| /// | `ShuttleActixWeb` | [shuttle-actix-web](https://crates.io/crates/shuttle-actix-web)| [actix-web](https://docs.rs/actix-web/4.3) | 4.3 | [GitHub](https://github.com/shuttle-hq/shuttle-examples/tree/main/actix-web/hello-world)| | |||
| /// | `ShuttleAxum` | [shuttle-axum](https://crates.io/crates/shuttle-axum) | [axum](https://docs.rs/axum/0.7) | 0.7 | [GitHub](https://github.com/shuttle-hq/shuttle-examples/tree/main/axum/hello-world) | | |||
| /// | `ShuttlePoem` | [shuttle-poem](https://crates.io/crates/shuttle-poem) | [poem](https://docs.rs/poem/2.0) | 2.0 | [GitHub](https://github.com/shuttle-hq/shuttle-examples/tree/main/poem/hello-world) | | |||
| /// | `ShuttleRama` | [shuttle-rama](https://crates.io/crates/shuttle-rama) | [rama](https://docs.rs/rama/0.2.0-alpha.5) | |||
There was a problem hiding this comment.
style: Missing version and example columns that other services have in the table. Add version '0.2.0-alpha.5' and GitHub example link to maintain consistency.
| async fn hello_world(mut stream: impl net::stream::Socket + net::Stream + Unpin) -> Result<(), Infallible> { | ||
| println!( | ||
| "Incoming connection from: {}", | ||
| stream | ||
| .peer_addr() | ||
| .map(|a| a.to_string()) | ||
| .unwrap_or_else(|_| "???".to_owned()) | ||
| ); |
There was a problem hiding this comment.
style: consider using debug! macro instead of println for better logging control in production
Description of change
Initial support of
ramahttps://ramaproxy.org/, using the latest alpha release0.2.0-alpha.5.First non-alpha release is expected Q1 2025, which will be
0.2.0.Rama can be used as a transport-layer runtime or application-layer runtime.
Rama users can also terminate their TLS themselves, but dunno if shuttle allows that?
Not a blocker if not (yet) possible, but just something that would at least have to be documented
in the
shuttle-ramaREADME.I also exposed the usual result public types, but similar to
tower,service types can become quite long due to the heavy use in nested generics.
As such probably I thought it would be easiest to just let them return
impl ShuttleServiceasthat keeps it clean, while still keeping rust happy and type-checked.
How has this been tested? (if applicable)
Not yet, please advice me on how to proceed.
It's my first real code contribution to shuttle so do let me know.