Add non-ui framework template #320
Conversation
| "http-server:distro:sys", | ||
| "http-client:distro:sys", | ||
| "vfs:distro:sys", | ||
| "homepage:homepage:sys" |
There was a problem hiding this comment.
Please sort these (and above) alphabetically. In general, if there's no reason for a specific order, alphabetize
| } else { | ||
| HyprEchoResp { payload: "don't talk to me".to_string() } | ||
| } | ||
| } |
There was a problem hiding this comment.
Probably the echo behaviors should match. Compare to the existing echo: https://github.com/hyperware-ai/kit/blob/master/src/new/templates/rust/no-ui/echo/echo/src/lib.rs
| #[derive(Serialize, Deserialize, Debug)] | ||
| pub struct HyprEchoResp { | ||
| payload: String, | ||
| } |
There was a problem hiding this comment.
I think these structs are to show that you can define custom types and have them work, but they feel a bit weird to me. Probably fine to keep
There was a problem hiding this comment.
In general, we want to demonstrate the best possible patterns we know of in these templates. Thats why this feels weird to me: its obviously not the right choice, and is merely there to demonstrate that you could do it. It'd feel a lot better if this was actually function (i.e. had multiple args or whatever)
There was a problem hiding this comment.
Another note: should not be called Req and Resp, since we are abstracting that away. Should be like "Args" and "Return Value" or smth
There was a problem hiding this comment.
I see what you mean: I rushed over being intentional with types/functions, I guess I was overindexing on showing these templates to an LLM (which is slightly different I guess to having it be a intentional template), since the assumption was that the code will be changed. I think I can do this slightly differently, and will ponder about it.
src/main.rs
Outdated
| @@ -1066,7 +1066,7 @@ async fn make_app(current_dir: &std::ffi::OsString) -> Result<Command> { | |||
| .short('t') | |||
| .long("template") | |||
| .help("Template to create") | |||
| .value_parser(["blank", "chat", "echo", "fibonacci", "file-transfer"]) | |||
| .value_parser(["blank", "chat", "echo", "fibonacci", "file-transfer", "hypr-echo"]) | |||
There was a problem hiding this comment.
Let's change name here and throughout to hyperapp-echo
We'll set a convention that all hyperapp templates are prefixed by hyperapp-
| process_macros = "0.1" | ||
| rmp-serde = "1.3.0" | ||
| serde_json = "1.0" | ||
| wit-bindgen = "0.36.0" |
There was a problem hiding this comment.
Get rid of unused deps (I suspect at least rmp-serde is unused)
|
I agree that it would be bad practice to merge this on develop with non-working code. I think you can just leave it open for now, since the only consumers of this right now are going to be me and Sam, so I believe we can hold off merging it until tests are done |
…type for clarity)
| #[derive(Serialize, Deserialize, Debug)] | ||
| pub struct Argument { | ||
| header: String, | ||
| body: String, |
There was a problem hiding this comment.
I wanted to put serde_json::Value as the type for the body field (to represent a generic JSON object, but I checked the WIT conversion and we don't handle this type of WIT type (I am not entirely sure whether its technically possible, but putting an arbitrary JSON object as an argument does seem useful.
There was a problem hiding this comment.
Yeah that definitely won't work. We always serialize to either a bytes string (with serde_json::to_vec()) or a string (serde_json::to_string()), so those are your two options. When we serialize the whole struct using JSON, we should choose to serialize body to a String because that will play nicer with JSON than an array of bytes
|
I got the tests to work (sort of)! Let me know what you think. The tests stuff was mostly an import issue (but a very nuanced one, I think it would be good to take a look into how I got the tests to work (how I imported different things) to let me know whether it's good or maybe we want to change some behaviors. Because SendResult is gone, we don't have access to |
|
Ok nice! I'm surprised that having the package-to-be-tested as a dependency doesn't get the caller-utils building in the proper place (i.e. within the tests' target dir). Will take a look tomorrow if at all possible. SendResult -> Result<R, AppSendError> should be a fairly straightforward replacement. Take a look at AppSendError here hyperware-ai/hyperprocess-macro#6 |
Problem
Lack of template for developing the new app framework processes.
Solution
Implemented a simple echo process.
Docs Update
TODO
Corresponding docs PR
Notes
Tests don't work right now. Probably should update the Hyperprocess framework to not point to a single commit (make it stable).