1fanwang opened a new pull request, #17894:
URL: https://github.com/apache/iceberg/pull/17894

   A caller that constructs `HiveCatalog` directly has to call `setConf` before 
`initialize`. Nothing enforces that order, and skipping it does not fail:
   
   ```java
   HiveCatalog catalog = new HiveCatalog();
   catalog.initialize("hive", properties);
   ```
   
   `initialize` finds no configuration, logs `No Hadoop Configuration was set, 
using the default environment Configuration`, and carries on against the 
ambient environment. Whatever the caller had prepared, the HMS thrift URIs, 
kerberos and SSL settings, is quietly gone, and the problem only shows up later 
when the catalog talks to the metastore.
   
   Getting it right today takes three lines in a fixed order:
   
   ```java
   HiveCatalog catalog = new HiveCatalog();
   catalog.setConf(configuration);
   catalog.initialize("hive", properties);
   ```
   
   `HadoopCatalog` already avoids this with [a convenience 
constructor](https://github.com/apache/iceberg/blob/1051b432aae7b27c5dd18a2b7b9a395b6694d84a/core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java#L138-L141).
 After this change `HiveCatalog` matches it, and the order can no longer be got 
wrong:
   
   ```java
   HiveCatalog catalog = new HiveCatalog(configuration, properties);
   ```
   
   ## What changes
   
   The new constructor takes a property map rather than a single warehouse 
string, because `HiveCatalog` reads more than one key:
   
   ```java
   public HiveCatalog(Configuration conf, Map<String, String> properties) {
     setConf(conf);
     initialize("hive", properties);
   }
   ```
   
   The Javadoc on `setConf` and `initialize` now points at it, so the one-step 
form is discoverable from either.
   
   ## Testing
   
   `testConstructorWithConfAndProperties` in `TestHiveCatalog` constructs a 
catalog through the new constructor, then asserts that `uri` and `warehouse` 
reach the Hadoop `Configuration` and that a caller-supplied entry set 
beforehand survives.
   
   With that test applied on top of `main` and `HiveCatalog.java` left 
unchanged:
   
   ```
   ./gradlew :iceberg-hive-metastore:compileTestJava
   
   TestHiveCatalog.java:316: error: constructor HiveCatalog in class 
HiveCatalog cannot be applied to given types;
   BUILD FAILED in 6s
   ```
   
   On this branch:
   
   ```
   ./gradlew :iceberg-hive-metastore:test --tests 
"org.apache.iceberg.hive.TestHiveCatalog.testConstructorWithConfAndProperties"
   
   BUILD SUCCESSFUL in 3m 1s
   tests=1 failures=0 errors=0 skipped=0
   PASS  testConstructorWithConfAndProperties()  time=0.433s
   ```
   
   Closes #16134.
   


-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to