HUE-8631 [hbase] pull thrift transport from hbase-site.xml

Review Request #13444 - Created Oct. 15, 2018 and submitted

Chris Conner
hue
master
HUE-8631
hue
enricoberti, jgauthier, johan, ranade, romain, roohi, weixia, yingc
commit ddc68621e93f6423103e1676ca53cf4fc1625c12
Author: Chris Conner <cconner@cloudera.com>
Date:   Mon Oct 15 15:18:10 2018 -0400

    HUE-8631 [hbase] pull thrift transport from hbase-site.xml

:100644 100644 12b1b78fbd... f6ed8887f2... M	apps/hbase/src/hbase/api.py
:100644 100644 c67eca2d97... 1f42fe5b5b... M	apps/hbase/src/hbase/conf.py
:100644 100644 d4abf7edb1... 8a49d4bbf1... M	apps/hbase/src/hbase/hbase_site.py

Tested with hbase-site.xml having hbase.regionserver.thrift.framed=true and hbase.regionserver.thrift.framed=false and missing hbase.regionserver.thrift.framed from hbase-site.xml. Note that CM just leaves hbase.regionserver.thrift.framed out of hbase-site.xml for buffered. This means the hue.ini needs the default to be buffered again instead of framed.

  • 0
  • 0
  • 7
  • 1
  • 8
Description From Last Updated
  1. Excellent!

    Just a bunch of nits, so that it is all consistent, and we could also easily add a config check when we know we are going to fail!

  2. apps/hbase/src/hbase/conf.py (Diff revision 2)
     
     

    nit: space before "'framed' is used t" ?

  3. apps/hbase/src/hbase/conf.py (Diff revision 2)
     
     

    Remove 'when CM does'?

    e.g.

    Default is buffered when not set in hbase-site.xml.

    Nit: final .

  4. apps/hbase/src/hbase/conf.py (Diff revision 2)
     
     

    Could we also keep the text in sync in

    https://github.com/cloudera/hue/blob/master/desktop/conf/pseudo-distributed.ini.tmpl

    https://github.com/cloudera/hue/blob/master/desktop/conf.dist/hue.ini

  5. apps/hbase/src/hbase/conf.py (Diff revision 2)
     
     

    nit:

    if get_thrift_transport() is buffered, raise a config check error has we know Hue does not supported it yet?

    1. It's actually framed that's not supported yet, buffered is the only config that works. Added check to make sure it's not framed.

    2. Just tested again to be certain, buffered works and framed does not.

  6. apps/hbase/src/hbase/hbase_site.py (Diff revision 2)
     
     

    Nit: , None is optional

  7. apps/hbase/src/hbase/hbase_site.py (Diff revision 2)
     
     

    nit:

    else:
    return ...

    1. Do we need another else here? we cover the 2 cases of hbase-site.xml and return in that if/else and then if not, we return THRIFT_TRANSPORT.get() which has a default of "buffered"?

  8. 
      
  1. Nice!

  2. apps/hbase/src/hbase/conf.py (Diff revision 3)
     
     

    nit here and in ini: about framed not being supported?

    "Should come from hbase-site.xml, do not set. 'framed' is used to chunk up responses, used with the nonblocking server in Thrift but is not supported in Hue."
    "'buffered' used to be the default of the HBase Thrift Server. Default is buffered when not set in hbase-site.xml."

  3. apps/hbase/src/hbase/hbase_site.py (Diff revision 3)
     
     

    nit: indeed, in general better to have a final 'else:', but in this case, it is fine to keep the last return without that way we are sure we have a return in any case (not 100% obvious to read the double ifs and internal returns)

  4. 
      
  1. Nice other import fixes!

  2. 
      
Review request changed

Status: Closed (submitted)

Loading...