Use a custom executor service if config is set #11
Conversation
...talog-client-common/src/main/java/com/amazonaws/glue/catalog/util/ThreadExecutorFactory.java
Outdated
Show resolved
Hide resolved
...talog-client-common/src/main/java/com/amazonaws/glue/catalog/util/ThreadExecutorFactory.java
Outdated
Show resolved
Hide resolved
khush-bhatia
changed the title
...talog-client-common/src/main/java/com/amazonaws/glue/catalog/util/ThreadExecutorFactory.java
Outdated
Show resolved
Hide resolved
...talog-client-common/src/main/java/com/amazonaws/glue/catalog/util/ThreadExecutorFactory.java
Outdated
Show resolved
Hide resolved
...talog-client-common/src/main/java/com/amazonaws/glue/catalog/util/ThreadExecutorFactory.java
Outdated
Show resolved
Hide resolved
...t-common/src/main/java/com/amazonaws/glue/catalog/metastore/GlueMetastoreClientDelegate.java
Outdated
Show resolved
Hide resolved
databricks-david-lewis
left a comment
databricks-david-lewis
left a comment
There was a problem hiding this comment.
LGTM! I think this is much cleaner and more extensible.
...t-common/src/main/java/com/amazonaws/glue/catalog/metastore/GlueMetastoreClientDelegate.java
Outdated
Show resolved
Hide resolved
...t-common/src/main/java/com/amazonaws/glue/catalog/metastore/GlueMetastoreClientDelegate.java
Outdated
Show resolved
Hide resolved
...t-common/src/main/java/com/amazonaws/glue/catalog/metastore/GlueMetastoreClientDelegate.java
Outdated
Show resolved
Hide resolved
khush-bhatia
changed the title
|
@ialek36 , The PR is ready for review. I am working on adding tests now, but hoping to get some preliminary feedback on the high-level solution. |
|
This looks great. LGTM. |
|
@aws-austin-lee , can you please review this PR. |
|
The changes themselves look good to me, but I do not see any unit tests. Can we have some test coverage, please? |
…ent-for-apache-hive-metastore into custom_thread_pool
|
@aws-austin-lee I have added unit tests. Could not run it locally though because I am running into this issue #1 |
|
@aws-austin-lee , I resolved the build issues and could run the tests locally. Can you please review ? Thanks. |
|
@ialek36 , the PR is ready for review. I have added unit tests as well. |
|
@khush-bhatia The unit test looks good to me. Let me follow up with @aws-austin-lee. I don't have merge permissions on the repo. |
|
Thanks for your contribution! |
6 participants
Issue #, if available:
Description of changes:
Created a new interface
ExecutorServiceFactoryand a default implementation of it that maintains existing behavior.Introduced a new config
hive.metastore.executorservice.factory.classthat will be used by Databricks to provide a custom executor service factory that returns a custom ExecutorService.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.