-
Notifications
You must be signed in to change notification settings - Fork 76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add personalized configuration parameters for each metastore. #315
Changes from 9 commits
d94d0a9
5b43c27
e9a93a1
394250d
14abc77
877a76a
085d7fc
cb7b7bf
8307404
f837b2e
bc64c92
8cb4b22
5bd959c
23258d2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,14 +51,15 @@ public abstract class AbstractMetaStore { | |
private List<String> writableDatabaseWhitelist; | ||
private List<String> mappedDatabases; | ||
private @Valid List<MappedTables> mappedTables; | ||
private Map<String, String> databaseNameMapping = Collections.emptyMap(); | ||
private transient Map<String, String> databaseNameMapping = Collections.emptyMap(); | ||
private @NotBlank String name; | ||
private @NotBlank String remoteMetaStoreUris; | ||
private @Valid MetastoreTunnel metastoreTunnel; | ||
private @NotNull AccessControlType accessControlType = AccessControlType.READ_ONLY; | ||
private transient @JsonProperty @NotNull MetaStoreStatus status = MetaStoreStatus.UNKNOWN; | ||
private long latency = 0; | ||
private transient @JsonIgnore HashBiMap<String, String> databaseNameBiMapping = HashBiMap.create(); | ||
private Map<String, String> configurationProperties;// = Collections.emptyMap(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isn't it nicer to instantiate as emptyMap so you don't need the null check? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @yangyuxia if you could revert or explain why this change was made I'm happy to approver once that's done,thanks! |
||
|
||
public AbstractMetaStore(String name, String remoteMetaStoreUris, AccessControlType accessControlType) { | ||
this.name = name; | ||
|
@@ -201,6 +202,15 @@ public HashBiMap<String, String> getDatabaseNameBiMapping() { | |
return databaseNameBiMapping; | ||
} | ||
|
||
public Map<String, String> getConfigurationProperties() { | ||
return configurationProperties; | ||
} | ||
|
||
public void setConfigurationProperties( | ||
Map<String, String> configurationProperties) { | ||
this.configurationProperties = configurationProperties; | ||
} | ||
|
||
@Transient | ||
public MetaStoreStatus getStatus() { | ||
return status; | ||
|
@@ -242,5 +252,4 @@ public String toString() { | |
.add("status", status) | ||
.toString(); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,10 +24,12 @@ | |
|
||
import static com.hotels.bdp.waggledance.api.model.AbstractMetaStore.newFederatedInstance; | ||
|
||
import java.util.Collections; | ||
import java.util.HashMap; | ||
import java.util.Map; | ||
import java.util.concurrent.TimeUnit; | ||
|
||
import com.hotels.bdp.waggledance.api.model.FederatedMetaStore; | ||
import org.apache.hadoop.hive.conf.HiveConf; | ||
import org.apache.hadoop.hive.conf.HiveConf.ConfVars; | ||
import org.junit.Before; | ||
|
@@ -68,8 +70,9 @@ public void setUp() { | |
@Test | ||
public void defaultFactory() { | ||
ArgumentCaptor<HiveConf> hiveConfCaptor = ArgumentCaptor.forClass(HiveConf.class); | ||
|
||
factory.newInstance(newFederatedInstance("fed1", THRIFT_URI)); | ||
FederatedMetaStore fed1 = newFederatedInstance("fed1", THRIFT_URI); | ||
fed1.setConfigurationProperties(Collections.singletonMap(ConfVars.METASTORE_KERBEROS_PRINCIPAL.varname, "hive/[email protected]")); | ||
factory.newInstance(fed1); | ||
verify(defaultMetaStoreClientFactory).newInstance(hiveConfCaptor.capture(), eq( | ||
"waggledance-fed1"), eq(3), eq(2000)); | ||
verifyNoInteractions(tunnelingMetaStoreClientFactory); | ||
|
@@ -80,6 +83,7 @@ public void defaultFactory() { | |
assertThat(hiveConf.getTimeVar(ConfVars.METASTORE_CLIENT_CONNECT_RETRY_DELAY, TimeUnit.SECONDS), is(5L)); | ||
assertThat(hiveConf.getBoolVar(ConfVars.METASTORE_USE_THRIFT_FRAMED_TRANSPORT), is(true)); | ||
assertThat(hiveConf.getBoolVar(ConfVars.METASTORE_USE_THRIFT_COMPACT_PROTOCOL), is(false)); | ||
assertThat(hiveConf.getVar(ConfVars.METASTORE_KERBEROS_PRINCIPAL), is("hive/[email protected]")); | ||
} | ||
|
||
@Test | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this had to change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yangyuxia if you could revert or explain why this change was made I'm happy to approver once that's done,thanks!