Remove explicit field syncing from Python client #2716
Conversation
| return self.table_service.grpc_table_op(self._fields[('scope', name)][1], table_op) | ||
| ticket = ticket_pb2.Ticket(ticket=f's/{name}'.encode(encoding='ascii')) | ||
|
|
||
| faketable = Table(session=self, ticket=ticket) |
There was a problem hiding this comment.
So - I wonder if we want to actually add abstractions, or safety barriers, such that returning faketable here makes sense. (For example, we need to make sure that we don't try to close() w/ a s/... ticket.)
Or may introduce ref_table(...) and fetch(), such that open_table(...) == ref_table(...).fetch()? (I think naming could be better.)
Of particular interest is the interest in obtaining ref_table(...).snapshot() without needing a FetchTableOp against the server.
There was a problem hiding this comment.
Or maybe, still have only open_table, but only execute the FetchTableOp when the user calls into a method that needs FetchTableOp-provided data?
There was a problem hiding this comment.
I think having open_table() still makes more sense if we consider the typical use case of a client wants to get a reference to a table in the server for the only reason to perform some operations on it. So lazy fetching isn't too meaningful here.
There was a problem hiding this comment.
I also think it would be more intuitive that the "this table doesn't exist" error shows up when you try to open the table, rather than the first time you try to use it. If we wanted lazy fetching, I think a separate open_lazy_table method would make more sense (and that can go into a separate PR if we thought it was useful).
There was a problem hiding this comment.
Ok, all fine by me. Will leave up to @jmao-denver for final approval.
Listing the available tables on the Python client now just performs a request to the server instead of trying to manage an entire cache of tables.
The
open_tablefunction now just constructs the ticket based on the table name and tries to open it directly.This means that all of the field-fetching and syncing stuff is gone.