Nomos chat app non interactive #567
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zeegomo
approved these changes
Jan 26, 2024
Comment on lines
-47
to
+55
| let bytes: Box<[u8]> = match (&self.data, &self.file) { | ||
| (Some(data), None) => data.clone().as_bytes().into(), | ||
| (None, Some(file_path)) => { | ||
| let file_bytes = std::fs::read(file_path)?; | ||
| file_bytes.into_boxed_slice() | ||
| } | ||
| (Some(_), Some(_)) => return Err("Cannot specify both data and file".into()), | ||
| (None, None) => return Err("Either data or file must be specified".into()), | ||
|
|
||
| let bytes: Box<[u8]> = if let Some(data) = &self.data { | ||
| data.clone().as_bytes().into() | ||
| } else { | ||
| let file_path = self.file.as_ref().unwrap(); | ||
| let file_bytes = std::fs::read(file_path)?; | ||
| file_bytes.into_boxed_slice() | ||
| }; | ||
|
|
There was a problem hiding this comment.
The previous solution, even being readable and concise, the panic message differed from the user friendly warning that Clap produced. The output when user provides the --author flag, but forgets the --message now is:
error: the following required arguments were not provided:
--message <MESSAGE>
Usage: nomos-cli chat --network-config <NETWORK_CONFIG> --node <NODE> --message <MESSAGE> --author <AUTHOR>
If user doesn't provide the author nor message, the app goes into the interactive mode
youngjoon-lee
approved these changes
Jan 29, 2024
There was a problem hiding this comment.
Looks good!
Bincode has no external cli tool to encode independantly, so either we need to write that tool, that we would be able to use from scripts, or we use chatapp to do encoding to ChatMessage directly, because it's specific to it.
in my opinion, it's good enough to do encoding in the chatapp, as you implemented.
al8n
pushed a commit
that referenced
this pull request
Mar 4, 2024
* Add option to send chat message non iteractively via nomos cli * Use clap to check if data or file is set * Require author if message flag set
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add params to nomos cli app to send chat message and exit the application. This will be used alongside the dissemination app to produce constant traffic in the testnet.
For chat app to decode messages, they need to be in ChatMessage struct, encoded with bincode. Bincode has no external cli tool to encode independantly, so either we need to write that tool, that we would be able to use from scripts, or we use chatapp to do encoding to ChatMessage directly, because it's specific to it.
Dissemination app will still be used for big data blob uploads to the testnet.