Add support for adding metadata/attachments #614
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
|
todo:
|
wkalt
commented
Sep 27, 2022
wkalt
commented
Sep 27, 2022
wkalt
commented
Sep 27, 2022
| die("Unexpected number of args") | ||
| } | ||
| filename := args[0] | ||
| err := utils.WithReader(ctx, filename, func(_ bool, rs io.ReadSeeker) error { |
There was a problem hiding this comment.
in general, the run commands in this patch are too heavy. It would be easier to test if these dispatched to standalone functions.
wkalt
force-pushed
the
wyatt/cli/add-metadata-attachment
branch
from
99c5027 to
870e338
Compare
wkalt
force-pushed
the
wyatt/cli/add-metadata-attachment
branch
from
870e338 to
e393a96
Compare
jhurliman
reviewed
Sep 27, 2022
go/cli/mcap/utils/utils.go
Outdated
| @@ -111,10 +115,152 @@ func FormatTable(w io.Writer, rows [][]string) { | |||
| tw.Render() | |||
| } | |||
|
|
|||
| func inferWriterOptions(info *mcap.Info) *mcap.WriterOptions { | |||
| // assume if there are no chunk indexes, the file is not chunked | |||
There was a problem hiding this comment.
Can this assumption fail on an unindexed file?
There was a problem hiding this comment.
if the file is chunked but not indexed, yeah it'll fail. I think though the only way to guard against that would be to scan the entire file to see if a chunk is present. I'm not sure that's worth it, but it's something we could do.
This is where it would be handy to have the writer options in the header.
jhurliman
approved these changes
Sep 27, 2022
wkalt
force-pushed
the
wyatt/cli/add-metadata-attachment
branch
from
52a1b5a to
d49a81d
Compare
Adds support for adding and getting metadata and attachments from an
existing mcap file. To add an attachment to a file,
mcap add attachment demo.mcap -f Makefile
To get an attachment out of the file,
mcap get attachment demo.mcap -n Makefile -o attachment.txt
To add metadata to a file,
mcap add metadata demo.mcap -k foo=bar -k bar=baz -n "my metadata"
To get metadata out of the file,
mcap get metadata demo.mcap -n "my metadata"
{
"bar": "baz",
"foo": "bar"
}
Also fixes a bug in the list attachments subcommand. Previously this was
listing chunks.
wkalt
force-pushed
the
wyatt/cli/add-metadata-attachment
branch
from
d49a81d to
1f66244
Compare
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 support for adding metadata/attachments
Adds support for adding and getting metadata and attachments from an
existing mcap file. To add an attachment to a file,
To get an attachment out of the file,
To add metadata to a file,
To get metadata out of the file,
Also fixes a bug in the list attachments subcommand. Previously this was
listing chunks.
Fixes: #434
Fixes: #372