Bigtable HappyBase enable/disable table should be a no-op

[tswast]

Currently methods which aren't available in Bigtable for HappyBase throw an NotImplementedError. https://github.com/GoogleCloudPlatform/gcloud-python/blob/master/gcloud/bigtable/happybase/connection.py#L392

This will make it more difficult to port applications to and from HBase. These operations should be no-ops (possibly logging a warning when run).

[tseaver]

@dhermes I'll defer to your judgement here: I'm not sure what would be worse for the developer porting an application: having unexpected NotImplementedError exceptions raised, or quasi-silent failures. Maybe we need to have an environment variable which toggles the exceptions on (or off)?

[dhermes]

@tswast I feel like not raising the exception would be misleading for the majority of people. Can you elaborate why you'd prefer the silent failure?

[tswast]

No-op is what the Java client does.

This is especially important for third-party tools that build on top of HBase to work on Cloud Bigtable with minimal modification.

I'm not sure if there are any similar tools that build on HappyBase that would have similar concerns, but ideally it should be as easy as possible to port an application to and from Cloud Bigtable and HappyBase.

@mbrukman Do you have an opinion about this?

[dhermes]

I'm not sure where a missing feature of the backend API falls into "easy as possible". As for minimal modification, they could simply remove calls to disable and enable table

[mbrukman]

Let's first agree in principle on the purpose of having a compatibility library such as the HBase client for Java and the HappyBase client for Python:

• our goal is to be compatible with code that uses HBase — this gets us instant integrations that work with HBase and user code that works with HBase becomes trivially portable
• as such, we should aim to have Bigtable become a drop-in replacement for HBase: code that works with HBase should be working with Bigtable with a tiny amount of configuration changes + code to connect to Bigtable
• we should avoid having to tell users to special-case Bigtable in their code that works with HBase, it should be transparent to the code as to whether it's working with HBase or Bigtable
• in essence, our library should hide the differences between HBase and Bigtable, and thus absorb complexity where possible, rather than exposing the differences and forcing customers to write 2 backend implementations, one for HBase and a separate one for Bigtable

Assuming we agree on the above, I think this argues that we should not throw exceptions, but either implement what we need to do, and make unnecessary functions no-ops whenever possible. Of course, each function/method should be considered on its own merits, what its intended semantics are, etc., but the goal here is not to reimplement the HBase semantics 1:1, but make it easy for code to use HBase and Bigtable transparently, with little to no work from the user.

AIUI, in HBase, before deleting a table or modifying some settings, you have to first disable it. Thus, all HBase-compliant code will always call disable_table() prior to calling delete_table().

I would argue that in this case, we should:

• convert calls to disable_table() and enable_table() to be no-ops rather than throw exceptions
• e.g., always return true to is_enabled()
• drop the warning from drop_table() being called with True for dropping table first
• document the difference that Bigtable tables have no concept of disabled state, because we can modify settings and delete tables without the prerequisite of disabling first

Note that this means that we would fail a test of the following type:

...
table.enable()
assertTrue(table.is_enabled())  # succeeds
table.disable()
assertFalse(table.is_enabled())  # fails

but I would argue this is a corner case and not something anyone should be doing in practice, outside of HBase tests themselves.

Other functions/methods should be considered on a case-by-case basis.

Let's consider the opposite: what do we gain from throwing exceptions? I am not sure.

What is the side effect of throwing exceptions?

Typically, this means that portability is hindered from HBase to Bigtable, leading to several options:

• users will either give up, or
• if they want to persevere, they will write abstraction libraries that separate HBase functions from Bigtable functions, or
• users fork gcloud-python and are unable to upgrade since they made local modifications

None of these are good outcomes, from my perspective, as they are providing a sub-par user experience, and we would be the cause of all of them.

Thus, I would advocate that we try to adapt our API to be functionally matching the intended semantics of HappyBase rather than trying to implement them strictly and throw exceptions where we are incompatible, because that breaks the spirit of the point of having a compatibility library in the first place.

Thoughts?

[dhermes]

Seems reasonable enough. I'll send out a PR.

[dhermes]

@mbrukman What about Connection.compact_table(...), Table.regions() and Table.counter_set(...)?

[mbrukman]

@dhermes:

• Connection.compact_table() is a performance optimization (compacted tables have lower depth, hence reads are faster). Bigtable does compactions on its own schedule, so we can safely ignore this as a noop. In essence, we're saying "sure, consider it done, we've taken care of all compactions for you". No semantics difference here.
• Table.regions(): doesn't look like we can return a reasonable value here: returning an empty list will look like it's broken, returning a single item with the first and last key will look like we always have a single region (which is false, and we don't want any application code to assume this is the case), but we also don't want to return the actual tablet server splits, either, so this may have to remain an exception, at least for now. If any user complains about this, please feel free to send them to me.
• Table.counter_set(): I think we should implement it the same way that HappyBase does, but looks like there was an explicit decision made (per the comments) to avoid this, so I may be missing context here. I see the point that it appears to defeat the purpose of a "counter", which I agree with, but presumably HappyBase users had a good (?) reason for adding this method, and it wouldn't break Bigtable or HappyBase to support it as-is. Who was the original author of this? We should chat about this further.

/cc: @sduskis, @garye, @lesv, @rameshdharan — thoughts?

[dhermes]

Want me to fold the Connection.compact_table() no-op into #1997? I am the original author of the gcloud-python port of HappyBase, did you mean https://github.com/wbolster/happybase/? As for counter_set in gcloud-python, I avoided it because I couldn't see a way to make it atomic. If there is a way, then I'm happy to implement it.

[tseaver]

For Table.counter_set(), see #1549. @mbrukman would CheckAndMutateRow (with an appropriate "row-is-empty" predicate) be atomic across both the check and the mutation?