feat: add Java client and server framework#69
Conversation
| import com.lancedb.lance.namespace.client.apache.model.CreateNamespaceRequest; | ||
| import com.lancedb.lance.namespace.client.apache.model.DropNamespaceRequest; | ||
| import com.lancedb.lance.namespace.client.apache.model.GetNamespaceRequest; | ||
| import com.lancedb.lance.namespace.client.apache.model.GetNamespaceResponse; | ||
| import com.lancedb.lance.namespace.client.apache.model.GetTableRequest; | ||
| import com.lancedb.lance.namespace.client.apache.model.GetTableResponse; | ||
| import com.lancedb.lance.namespace.client.apache.model.ListNamespacesRequest; | ||
| import com.lancedb.lance.namespace.client.apache.model.ListNamespacesResponse; | ||
| import com.lancedb.lance.namespace.client.apache.model.NamespaceExistsRequest; | ||
| import com.lancedb.lance.namespace.client.apache.model.RegisterTableRequest; | ||
| import com.lancedb.lance.namespace.client.apache.model.RegisterTableResponse; | ||
| import com.lancedb.lance.namespace.client.apache.model.TableExistsRequest; |
There was a problem hiding this comment.
Can we refactor (or re-generate) the package to be com.lancedb.lance.namspace.model.XXXX?
There was a problem hiding this comment.
let me do that in another PR then, this change will make this PR too many changes.
| import com.lancedb.lance.namespace.client.apache.model.RegisterTableResponse; | ||
| import com.lancedb.lance.namespace.client.apache.model.TableExistsRequest; | ||
|
|
||
| public interface LanceNamespace { |
There was a problem hiding this comment.
Add some TODO to remember to add comments?
| converted.setNamespace(response.getNamespace()); | ||
| converted.setName(response.getName()); | ||
| converted.setProperties(response.getProperties()); |
There was a problem hiding this comment.
Can it be generated automatically? If we add a property to the spec, do we also need to add a setter here manually?
There was a problem hiding this comment.
currently yes, because they are different client & server models.
| import com.lancedb.lance.namespace.server.springboot.model.GetNamespaceResponse; | ||
| import com.lancedb.lance.namespace.server.springboot.model.GetTableResponse; | ||
| import com.lancedb.lance.namespace.server.springboot.model.ListNamespacesResponse; | ||
| import com.lancedb.lance.namespace.server.springboot.model.RegisterTableResponse; |
There was a problem hiding this comment.
Can we find a way to put the req/resp entities under the same packages?
There was a problem hiding this comment.
need to do some experiments with that, let me check if that is feasible
There was a problem hiding this comment.
https://openapi-generator.tech/docs/generators/java
https://openapi-generator.tech/docs/generators/spring
Looks like there is not a way to let them be the same model, because both always generate models... so some sort of conversion is necessary to work between server and client models although they are exactly the same.
There was a problem hiding this comment.
In that case we probably need to add some enforcement that spec changes must also update client and server model mapping.
github-actions
Bot
added
python
|
@yanghua fixed the exists API inconsistency by returning a proper |
yanghua
left a comment
yanghua
left a comment
There was a problem hiding this comment.
Overall, LGTM. Let's iterate in the further PR if we find issues.
2 participants
This PR introduces the basic client and server interactions expected in Lance namespace:
LanceNamespaceinterface to support JavaLanceRestNamespaceas example, but other implementation likeLanceHiveNamespacecan also be provided as separated modules.delegate, which can be updated later to be based on configs, for example if user configshive, then the server can initiate aLanceHiveNamespaceand delegate executions all to it. This will directly provides a REST server implementation for Hive.