[jira] Commented: (ZOOKEEPER-38) headers (version+) in log/snap files

["Mahadev konar (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/ZOOKEEPER-38?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12611885#action_12611885 ] 

Mahadev konar commented on ZOOKEEPER-38:
----------------------------------------

the patch looks good --- here are some of my comments --- 

also I am not finished reviewing it :). its a long patch --- 

0) overall there are too many interfaces in the patch. though i uinderstand that interfaces make things pluggable but I think we should restrict only wehn we need it and create interfaces only when we seriously think that it would be good to have it pluggable.

1) DataTereeBuilder is an interface and has only one subclass --- i think that we should not make this pluggable and just have a datatree class instead of the interfaces and implementing classes.
2) FileDBINFO implements DBINFO but  FILEDBINFO Is the only subclass of DBINFO. I think here again we should just keep it simple and not make this an interface and pluggable.
3) Snapshotting plicy is again pluggable -- we should also not make this pluggable and let it work our default implementation.
4) we have arguments passing around as url though there are not true url's in java -- but just strings. if we really want to make this pluggable and use url's then we should use URL class in java and use its methods for checking what kind of provider we want it to return. like create a URL(string) and url.getProtocol.equals("file") then we retrun our default one.
5) i dont see any use case of more persistence layers as -D expressions? what if we had something like this 

-Dpersisten.provider=classname and then just use reflections to create the provider class else we just return the default provider. I think it makes the code simpler.

i am not done reviewing the patch entirely. :) 


> headers (version+) in log/snap files
> ------------------------------------
>
>                 Key: ZOOKEEPER-38
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-38
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: server
>            Reporter: Patrick Hunt
>            Assignee: Andrew Kornev
>             Fix For: 3.0.0
>
>         Attachments: ZOOKEEPER-38.patch, ZOOKEEPER-38.patch
>
>
> Moved from SourceForge to Apache.
> http://sourceforge.net/tracker/index.php?func=detail&aid=1961767&group_id=209147&atid=1008547

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.