Bigtable: Separate row types to remove confusion around return types of row.commit #8662
Conversation
googlebot
added
the
cla: yes
tseaver
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
tseaver
added
api: bigtable
yoshi-kokoro
removed
the
kokoro:force-run
| @@ -199,6 +207,60 @@ def row(self, row_key, filter_=None, append=False): | |||
| else: | |||
| return DirectRow(row_key, self) | |||
|
|
|||
| def append_row(self, row_key): | |||
| """Create a :class:`~google.cloud.bigtable.row.AppendRow` associated with this table. | |||
There was a problem hiding this comment.
We should make sure these comments reflect why you would choose one of the rows over another.
There was a problem hiding this comment.
Not sure, I'm getting. You want something like: you read method's docs, and you don't need to read row class'es docs, 'cause method's docs will tell you everything you need? So classes should be described a little in method's docs?
|
@billyjacobson can you take a look and see if you think this improves the usability of the python client? |
| row2_obj = table.conditional_row(row_keys[1], filter_=filter_) | ||
| # [END bigtable_table_conditional_row] | ||
|
|
||
| row1_obj.set_cell(COLUMN_FAMILY_ID, COL_NAME1, CELL_VAL1, state=False) |
There was a problem hiding this comment.
@crwilcox I feel like instead of state, we could do something like onMatch=False or onMatch=True
There was a problem hiding this comment.
@billyjacobson that seems reasonable to me. Does that resemble something we do in other languages? I agree state isn't a great name, but it would be good if we didn't diverge from a cross-language pattern if there is one.
There was a problem hiding this comment.
Node calls it onMatch and onNoMatch.
The RPCs call it true_mutations and false_mutations
Go uses mtrue, mfalse
PHP trueMutations, falseMutations
Ruby true_mutations, false_mutations
So probably true and false mutations would be most consistent
There was a problem hiding this comment.
We can't just rename the state parameter: it would be a breaking change.
AVaksman
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
IlyaFaer
changed the title
Looks like some consensus have been achieved on #8451, so here is new PR