[
https://issues.apache.org/jira/browse/HCATALOG-404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13270926#comment-13270926
]
[email protected] commented on HCATALOG-404:
--------------------------------------------------------
bq. On 2012-05-08 16:41:27, David Capwell wrote:
bq. > If you are finally going Configuration over Properties can we just pass
around a configuration object and not a Properties object? Stuff like this
really bothers me still
https://github.com/apache/hcatalog/blob/trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java#L82
I was hoping to make deal with the external config issue and rewire the
internals later. Less burden on the reviewers given our schedule :-). I'll see
if I can get it done in this patch.
bq. On 2012-05-08 16:41:27, David Capwell wrote:
bq. >
storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerConfiguration.java,
line 64
bq. > <https://reviews.apache.org/r/5062/diff/1/?file=107664#file107664line64>
bq. >
bq. > Why use merge when you can just use the Configuration constructor?
The constructor should also be faster.
It's for cases where I'd like to modify an instance as opposed to creating a
new one.
bq. On 2012-05-08 16:41:27, David Capwell wrote:
bq. >
storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpointClient.java,
line 44
bq. > <https://reviews.apache.org/r/5062/diff/1/?file=107666#file107666line44>
bq. >
bq. > Why switch from meta to root, is this just so it only runs on one
server?
Yep, singleton. It'll make things more predictable. Later on we can revisit
this.
bq. On 2012-05-08 16:41:27, David Capwell wrote:
bq. > storage-handlers/hbase/build.xml, line 342
bq. > <https://reviews.apache.org/r/5062/diff/1/?file=107661#file107661line342>
bq. >
bq. > I avoid ant as much as possible so this might be my ignorance but
shouldn't that just be **/config/*?
bq. >
bq. > Also what files are in the jar? By convention the -default.xml
should be in the jar but -site.xml can not. Is this true for the build?
That just means do it recursively.
-default.xml is in the jar. -site is part of the package.
bq. On 2012-05-08 16:41:27, David Capwell wrote:
bq. >
storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerConfiguration.java,
line 15
bq. > <https://reviews.apache.org/r/5062/diff/1/?file=107670#file107670line15>
bq. >
bq. > Can you add a test that will use a RM stub, put that class in the
config, then pass that config to the RM factory and check the class? Should
verify that the conversion from config -> properties is working properly.
ok, will weight this test against dropping properties.
bq. On 2012-05-08 16:41:27, David Capwell wrote:
bq. > storage-handlers/hbase/src/resources/revision-manager-default.xml, lines
22-35
bq. > <https://reviews.apache.org/r/5062/diff/1/?file=107668#file107668line22>
bq. >
bq. > Why only these two defaults and not all the other found in RM?
bq. >
bq. > public static final String DEFAULT_DATADIR = "/revision-management";
bq. > public static final String DEFAULT_HOSTLIST = "localhost:2181";
bq. > private static int DEFAULT_WRITE_TRANSACTION_TIMEOUT = 14400000;
bq. >
bq. > just to list a few
Hmmm, I'm actually hesitant about having defaults in more than one place. Will
have to rethink this.
- Francis
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5062/#review7686
-----------------------------------------------------------
On 2012-05-08 05:45:38, Francis Liu wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/5062/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-05-08 05:45:38)
bq.
bq.
bq. Review request for hcatalog, Vandana Ayyalasomayajula and Thomas.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. revision manager currently expects some configs to be included in
hbase-site.xml, which complicates deployment as well as adding unrelated
properties which are not native hbase.
bq.
bq. I've added a new class and revision-*.xml artifacts.
bq.
bq.
bq. This addresses bug HCATALOG-404.
bq. https://issues.apache.org/jira/browse/HCATALOG-404
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. storage-handlers/hbase/build.xml 6653a66
bq. storage-handlers/hbase/conf/revision-manager-site.xml PRE-CREATION
bq.
storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseHCatStorageHandler.java
d3faa04
bq.
storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerConfiguration.java
PRE-CREATION
bq.
storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java
55c97e0
bq.
storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpointClient.java
3e03909
bq.
storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java
cffcf5e
bq. storage-handlers/hbase/src/resources/revision-manager-default.xml
PRE-CREATION
bq. storage-handlers/hbase/src/test/log4j.xml 8fd4fc9
bq.
storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerConfiguration.java
PRE-CREATION
bq.
storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java
1e94fd2
bq.
bq. Diff: https://reviews.apache.org/r/5062/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq.
bq. Thanks,
bq.
bq. Francis
bq.
bq.
> add separate configuration for revision manager
> -----------------------------------------------
>
> Key: HCATALOG-404
> URL: https://issues.apache.org/jira/browse/HCATALOG-404
> Project: HCatalog
> Issue Type: Improvement
> Affects Versions: 0.4.1
> Reporter: Francis Liu
> Assignee: Francis Liu
> Attachments: HCATALOG-404.patch
>
>
> revision manager currently expects some configs to be included in
> hbase-site.xml, which is complicates deployment and adding unrelated
> properties which are not native hbase.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
