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 backend API support for "vng" input #4251

Closed
philrz opened this issue Dec 6, 2022 · 1 comment · Fixed by #4345
Closed

Add backend API support for "vng" input #4251

philrz opened this issue Dec 6, 2022 · 1 comment · Fixed by #4345
Assignees

Comments

@philrz
Copy link
Contributor

philrz commented Dec 6, 2022

Repro is with Zed commit 723a6e9.

VNG can currently be loaded via -i vng with the Zed CLI tooling. However, the Zed CLI tooling currently handles all input by first turning it into ZNG within the client and posting it to the backend as ZNG. Meanwhile, the Brim app and lake API both post data in native formats and rely on the backend to read in that format. Therefore to have feature parity between our different clients & API it would be ideal to add VNG load support in the backend as well.

How things look at the moment in Zed commit 723a6e9 and the attached test data sample.vng.gz (after uncompressing):

$ zed -version
Version: v1.3.0-15-g723a6e9c

$ curl http://localhost:9867/version
{"version":"v1.3.0-15-g723a6e9c"}

$ zed create foo
pool created: foo 2IYRIdBFRfKc5p393fLfne5ZqXS

$ zed use foo
Switched to branch "main" on pool "foo"

$ curl -X POST -H 'Accept: application/json' -H 'Content-Type: application/x-vng' -d @sample.vng http://localhost:9867/pool/foo/branch/main
Warning: Couldn't read data from file "sample.vng", this makes an empty POST.
{"type":"Error","kind":"invalid operation","error":"unsupported MIME type: application/x-vng"}

In addition to adding the MIME type, @nwt is aware of what else is needed:

zio/vngio needs some rejiggering to make it work. I have the changes and just need to put up a PR.

@philrz philrz changed the title Add backend API support for "zst" input Add backend API support for "vng" input Dec 12, 2022
nwt added a commit that referenced this issue Jan 30, 2023
* Send a VNG response for an API request whose negotiated content type
  is application/x-vng.

* When loading data via POST /pool/{pool}/branch/{branch}, if the
  request content type is application/x-vng, copy the request body to a
  temporary file for the benefit of the VNG reader, which needs a reader
  that implements io.ReaderAt and io.Seeker.

Closes #4251.
nwt added a commit that referenced this issue Jan 30, 2023
* Send a VNG response for an API request whose negotiated content type
  is application/x-vng.

* When loading data via POST /pool/{pool}/branch/{branch}, if the
  request content type is application/x-vng, copy the request body to a
  temporary file for the benefit of the VNG reader, which needs a reader
  that implements io.ReaderAt and io.Seeker.

Closes #4251.
@nwt nwt closed this as completed in #4345 Jan 31, 2023
nwt added a commit that referenced this issue Jan 31, 2023
* Send a VNG response for an API request whose negotiated content type
  is application/x-vng.

* When loading data via POST /pool/{pool}/branch/{branch}, if the
  request content type is application/x-vng, copy the request body to a
  temporary file for the benefit of the VNG reader, which needs a reader
  that implements io.ReaderAt and io.Seeker.

Closes #4251.
@philrz
Copy link
Contributor Author

philrz commented Feb 3, 2023

Verified in Zed commit 743eebd using the attached sample.vng.gz.

$ zed -version
Version: v1.5.0-15-g743eebda

$ curl http://localhost:9867/version
{"version":"v1.5.0-15-g743eebda"}

$ zed create foo
pool created: foo 2LFAmQ2WKwq53ZGKcBVpJlmkbRW

$ curl -X POST -H 'Accept: application/json' -H 'Content-Type: application/x-vng' --data-binary @sample.vng.gz http://localhost:9867/pool/foo/branch/main
{"commit":"0x106b22ca09a250b84af5ad9481074ff884e57c7c","warnings":[]}

Using this to add VNG support in the app is tracked in brimdata/zui#2650.

Thanks @nwt!

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