Implemented Remaining Catalog operations for REST catalog #240
Conversation
|
@chil-pavn In general this is looking good, I'll give it a more in-depth review in the next couple days. Could you add some unit tests for these as we have for the other functions? (basically just mocking out the server responses with a local http server) |
| payload := map[string]interface{}{ | ||
| "source": map[string]interface{}{ | ||
| "namespace": strings.Split(fromNs, namespaceSeparator), | ||
| "name": fromTbl, | ||
| }, | ||
| "destination": map[string]interface{}{ | ||
| "namespace": strings.Split(toNs, namespaceSeparator), | ||
| "name": toTbl, | ||
| }, | ||
| } |
There was a problem hiding this comment.
any reason not to create a struct for this instead of the maps?
| for k, v := range ret.Config { | ||
| tblProps[k] = v | ||
| } |
There was a problem hiding this comment.
maps.Copy(tblProps, ret.Config)?
|
Hey @zeroshade , it appears there was already PR #146 for the same table operations, which also got merged. Should i work on the unit tests, as i could see that was not included in the PR? |
|
@chil-pavn that would be fantastic thanks! |
|
@zeroshade Sure, I will take that up. Also, it would be very helpful if i we link PRs to the respected issues. |
|
Ideally we should definitely be doing that, I'll definitely admit to my own mistakes in not doing so lately. :( |
Part of #63 . Tried implementing createTable, dropTable and renameTable methods taking reference to loadTable and rest-catalog-open-api.