[
https://issues.apache.org/jira/browse/SOLR-9242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15361830#comment-15361830
]
Hrishikesh Gadre commented on SOLR-9242:
----------------------------------------
[~varunthacker] Thanks for the review comments. The updated patch looks good
mostly. Only couple of minor comments,
bq. This prop solr.hdfs.confdir in the solr.xml file never seemed to be
getting used? I removed it and the tests pass. Do we need this?
The HdfsBackupRepository uses HdfsDirectory internally. This optional property
is being used by the HdfsDirectoryFactory. My guess is that in the test
environment Hadoop conf directory is not created/initialized. I don't think
there is any harm in keeping this configuration option.
bq. Changed setRepository(Optional<String> repository) to setRepository(String
repository . Seems cleaner from an API perspective given it's a setter.
I am OK with changing the setRepository parameter from Optional<String> to
String. But it looks like you have changed the attribute type as well. Since we
are using Java 8 now, we should use Optional type to clearly specify optional
attributes. This helps to improve the readability of the code. Here is a good
article about the Java 8 Optional type.
http://blog.codefx.org/techniques/intention-revealing-code-java-8-optional/
How about following?
- The parameter type of setter should be String. The setter method should
initialize the attribute based on its nullability.
- The type of attribute as well as the getter should be Optional<String>. This
clearly indicates that the attribute is optional (without having to read the
code comment in the CollectionAdminRequest class).
- In the getParams method - replace the null check with isPresent() method call.
> Collection level backup/restore should provide a param for specifying the
> repository implementation it should use
> -----------------------------------------------------------------------------------------------------------------
>
> Key: SOLR-9242
> URL: https://issues.apache.org/jira/browse/SOLR-9242
> Project: Solr
> Issue Type: Improvement
> Reporter: Hrishikesh Gadre
> Assignee: Varun Thacker
> Attachments: SOLR-9242.patch, SOLR-9242.patch, SOLR-9242.patch,
> SOLR-9242.patch
>
>
> SOLR-7374 provides BackupRepository interface to enable storing Solr index
> data to a configured file-system (e.g. HDFS, local file-system etc.). This
> JIRA is to track the work required to extend this functionality at the
> collection level.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
