[GitHub] [flink] wuchong commented on pull request #12146: [FLINK-17968][hbase] Fix Hadoop Configuration is not properly serialized in HBaseRowInputFormat

[GitBox <gi...@apache.org>]

wuchong commented on pull request #12146:
URL: https://github.com/apache/flink/pull/12146#issuecomment-635064897


   Thanks for the contribution @Tartarus0zm . I ajusted the title to the corresponding JIRA issue. 
   
   Regarding to the fix, I agree with @rmetzger , we should use `HBaseConfigurationUtil.serializeConfiguration()` to serialize the conf, because there is not only the zk information in it. You can refer to `HBaseLookupFunction` about how to do it. 
   
   Besides, `HBaseRowDataInputFormat` is copied from `HBaseRowInputFormat` which has the same problem. It would be nice if you can fix it too. 
   
   Another thing is about the testing, the current IT case can pass is because it adds the hbase site xml to the classloader which is not correct in a real environment. I think we should remove [`registerHBaseMiniClusterInClasspath`](https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-hbase/src/test/java/org/apache/flink/connector/hbase/util/HBaseTestingClusterAutoStarter.java#L181).
   
   A tip: tests in flink-connector-hbase module can't run in IDEA because of class conflicts, but can run using mvn command. You can use `mvn test -Dtest=org.apache.flink.connector.hbase.HBaseConnectorITCase -Dcheckstyle.skip=true` in `flink-connector-hbase` directory to run individual tests (please mvn install the project before running this command). 
   
   
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org