feat: Implement create table and update table api for rest catalog.#97
feat: Implement create table and update table api for rest catalog.#97Fokko merged 10 commits intoapache:mainfrom
Conversation
Fokko
left a comment
There was a problem hiding this comment.
Some small comments, and there is a lot in here. As I mentioned earlier, having the REST Docker images would make testing a lot easier. Apart from that it, looks good!
| ("total-position-deletes", "0"), | ||
| ("total-equality-deletes", "0") | ||
| ].iter().map(|p|(p.0.to_string(), p.1.to_string()))) | ||
| ("spark.app.id", "local-1646787004168"), |
There was a problem hiding this comment.
Nit: shouldn't this be caught by the linter? Same for the trailing comma's below.
There was a problem hiding this comment.
Sorry, I don't quite get your point.
There was a problem hiding this comment.
Why is this changed? It looks only indentation has changed, and most linters would not allow that since it makes the linter non-deterministic.
There was a problem hiding this comment.
Seems rust-fmt not enforcing the indentation, let me do some check.
I've tracked this in #100, and will do it after this get merged. This pr is already too large for me to add more stuff. |
|
cc @Fokko I've fixed comment, PTAL |
|
cc @Xuanwo Any other comments? |
LGTM, let's go! |
Fokko
left a comment
There was a problem hiding this comment.
Looking great @liurenjie1024 Thanks for working on this!
…pache#97) * feat: Implement update table/create table api for rest catalog * Add create table test * Add some tests for update table * Add tests for table updates and requirements * Add some comments * Add tests for transaction * Fix format * Fix linters * Stop upgrading to uuid 1.6.0 * Fix comments
Close #60
Last part of first version of rest catalog, add create table api and update table api.
Also introduces transaction api.