Skip to content
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 commands for command records #33

Closed
shikhar opened this issue Nov 25, 2024 · 5 comments · Fixed by #46
Closed

add commands for command records #33

shikhar opened this issue Nov 25, 2024 · 5 comments · Fixed by #46
Assignees
Milestone

Comments

@shikhar
Copy link
Member

shikhar commented Nov 25, 2024

Acts on a stream, I reckon something like...

.. trim <trim-point>

.. fence <token>

SDK exposes a CommandRecord type, this would utilize that to prepare a record to append

@shikhar
Copy link
Member Author

shikhar commented Nov 28, 2024

@vrongmeal also think about how to display command records when reading, maybe stderr-only? a TryFrom<..> for CommandRecord?

@vrongmeal
Copy link
Member

@shikhar by .. trim <trim-point> do you mean the .. is an indicator of an internal command? Or am I missing something?

If yes, we can simply have:

.trim <trim-point>
.fence <token>

Similar to sqlite ^

I was also thinking if we have a way to add headers to an append record. Maybe multiple "header" command messages followed by the actual message.

.header KEY1=VAL1
.header KEY2=VAL2
message

The above would evaluate as:

AppendRecord::new("message")
  .with_headers(vec![
    Header::new("KEY1", "VAL1"),
    Header::new("KEY2", "VAL2"),
  ])

@vrongmeal
Copy link
Member

also think about how to display command records when reading, maybe stderr-only? a TryFrom<..> for CommandRecord?

Didn't catch this. When do we want to display a command record?

@shikhar
Copy link
Member Author

shikhar commented Dec 3, 2024

Ah no, I had meant these would be stream-level commands that result in a unary append() request with the command record. So .. was a plceholder for s2 stream $basin $stream.

I don't want to be further interpreting the newline-delimited plaintext data we currently support. We do need to consider alternate formats that allow encoding headers as well as binary data, but let's do that separately.

@shikhar
Copy link
Member Author

shikhar commented Dec 3, 2024

also think about how to display command records when reading, maybe stderr-only? a TryFrom<..> for CommandRecord?

Didn't catch this. When do we want to display a command record?

Command records have binary payloads - so we can't display them with the current interpretation read makes of treating all records as plaintext. I was suggesting we can skip over displaying them on stdout, and only on stderr.

@shikhar shikhar added this to the M1 milestone Dec 3, 2024
vrongmeal added a commit that referenced this issue Dec 4, 2024
Resolves: #33

Signed-off-by: Vaibhav Rabber <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants