Uh oh!
There was an error while loading. Please reload this page.
STORM-2508:storm-solr enhancement: update solrj to 5.5, support custom SolrClientFactory and commit operation - #2108
Closed
aylei wants to merge 1 commit into
Closed
STORM-2508:storm-solr enhancement: update solrj to 5.5, support custom SolrClientFactory and commit operation#2108aylei wants to merge 1 commit into
aylei wants to merge 1 commit into
Conversation
storm-solr enhancement: update solrj to 5.5, support custom SolrClientFactory and commit operation
srdo
reviewed
Aug 12, 2017
srdo
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the contribution @AleiHanami. I left a few comments, but the changes look reasonable. Could you open a PR against master as well, since we need changes to go there first?
| @@ -0,0 +1,30 @@ | |||
| /* | |||
| * DefaultCloudSolrClientFactory.java | |||
Contributor
There was a problem hiding this comment.
Please replace this license header with the Apache header (copy it from one of the other files)
| * default implementation of {@link SolrClientFactory}, which takes a zookeeper host | ||
| * and produce {@link org.apache.solr.client.solrj.impl.CloudSolrClient} with the host. | ||
| * | ||
| * @author alei |
| import java.util.Map; | ||
| /** | ||
| * <p>this Class build the {@link Schema} by Solr Schema API.</p> |
Contributor
There was a problem hiding this comment.
Is there any reason to prefer the REST schema builder over this one? Otherwise maybe we should remove the other one?
| Schema converted = new Schema(); | ||
| converted.setUniqueKey(representation.getUniqueKey()); | ||
| converted.setName(representation.getName()); | ||
| converted.setVersion(String.valueOf(representation.getVersion())); |
Contributor
There was a problem hiding this comment.
Nit: If this is a float, shouldn't we keep it that way in Schema?
| FieldType fieldType = new FieldType(); | ||
| fieldType.setClazz(type.getAttributes().get("class").toString()); | ||
| if (type.getAttributes().get("multiValued") != null) { | ||
| fieldType.setMultiValued(Boolean.valueOf(type.getAttributes().get("multiValued").toString())); |
Contributor
There was a problem hiding this comment.
Is this attribute already a boolean? If so I don't think there's a reason to toString and then Boolean.valueOf it.
| new ArrayList<>(representation.getFields().size()); | ||
| for (Map<String, Object> field : representation.getFields()) { | ||
| org.apache.storm.solr.schema.Field schemaField = new org.apache.storm.solr.schema.Field(); | ||
| schemaField.setName(field.get("name").toString()); |
Contributor
There was a problem hiding this comment.
Nit: An explicit cast to String might be nicer here, so you get an exception in case this isn't a String for some reason.
| return schema; | ||
| } | ||
| private Schema convert(final SchemaRepresentation representation) { |
Contributor
There was a problem hiding this comment.
Nit: It might be good to add a unit test for this, to verify that this conversion does the right thing.
| */ | ||
| public interface CommitCallback extends Serializable { | ||
| void process(SolrClient solrClient, String collection) throws SolrServerException, IOException; |
Contributor
There was a problem hiding this comment.
Nit: Shouldn't this be named "commit"?
| <groupId>org.apache.solr</groupId> | ||
| <artifactId>solr-solrj</artifactId> | ||
| <version>5.2.1</version> | ||
| <version>5.5.0</version> |
There was a problem hiding this comment.
5.5.0 version is affected by a security problem and is also more than 1.5 years old.
http://lucene.apache.org/solr/news.html
Please consider moving to Solr 6.5.1 or 6.6.0
| <groupId>org.apache.solr</groupId> | ||
| <artifactId>solr-test-framework</artifactId> | ||
| <version>5.2.1</version> | ||
| <version>5.5.0</version> |
There was a problem hiding this comment.
To avoiding adding version everywhere, this should be made a property
d2r pushed a commit
to d2r/storm
that referenced
this pull request
Oct 16, 2018
We are closing stale Pull Requests to make the list more manageable. Please re-open any Pull Request that has been closed in error. Closesapache#608Closesapache#639Closesapache#640Closesapache#648Closesapache#662Closesapache#668Closesapache#692Closesapache#705Closesapache#724Closesapache#728Closesapache#730Closesapache#753Closesapache#803Closesapache#854Closesapache#922Closesapache#986Closesapache#992Closesapache#1019Closesapache#1040Closesapache#1041Closesapache#1043Closesapache#1046Closesapache#1051Closesapache#1078Closesapache#1146Closesapache#1164Closesapache#1165Closesapache#1178Closesapache#1213Closesapache#1225Closesapache#1258Closesapache#1259Closesapache#1268Closesapache#1272Closesapache#1277Closesapache#1278Closesapache#1288Closesapache#1296Closesapache#1328Closesapache#1342Closesapache#1353Closesapache#1370Closesapache#1376Closesapache#1391Closesapache#1395Closesapache#1399Closesapache#1406Closesapache#1410Closesapache#1422Closesapache#1427Closesapache#1443Closesapache#1462Closesapache#1468Closesapache#1483Closesapache#1506Closesapache#1509Closesapache#1515Closesapache#1520Closesapache#1521Closesapache#1525Closesapache#1527Closesapache#1544Closesapache#1550Closesapache#1566Closesapache#1569Closesapache#1570Closesapache#1575Closesapache#1580Closesapache#1584Closesapache#1591Closesapache#1600Closesapache#1611Closesapache#1613Closesapache#1639Closesapache#1703Closesapache#1711Closesapache#1719Closesapache#1737Closesapache#1760Closesapache#1767Closesapache#1768Closesapache#1785Closesapache#1799Closesapache#1822Closesapache#1824Closesapache#1844Closesapache#1874Closesapache#1918Closesapache#1928Closesapache#1937Closesapache#1942Closesapache#1951Closesapache#1957Closesapache#1963Closesapache#1964Closesapache#1965Closesapache#1967Closesapache#1968Closesapache#1971Closesapache#1985Closesapache#1986Closesapache#1998Closesapache#2031Closesapache#2032Closesapache#2071Closesapache#2076Closesapache#2108Closesapache#2119Closesapache#2128Closesapache#2142Closesapache#2174Closesapache#2206Closesapache#2297Closesapache#2322Closesapache#2332Closesapache#2341Closesapache#2377Closesapache#2414Closesapache#2469
d2r pushed a commit
to d2r/storm
that referenced
this pull request
Oct 16, 2018
We are closing stale Pull Requests to make the list more manageable. Please re-open any Pull Request that has been closed in error. Closesapache#608Closesapache#639Closesapache#640Closesapache#648Closesapache#662Closesapache#668Closesapache#692Closesapache#705Closesapache#724Closesapache#728Closesapache#730Closesapache#753Closesapache#803Closesapache#854Closesapache#922Closesapache#986Closesapache#992Closesapache#1019Closesapache#1040Closesapache#1041Closesapache#1043Closesapache#1046Closesapache#1051Closesapache#1078Closesapache#1146Closesapache#1164Closesapache#1165Closesapache#1178Closesapache#1213Closesapache#1225Closesapache#1258Closesapache#1259Closesapache#1268Closesapache#1272Closesapache#1277Closesapache#1278Closesapache#1288Closesapache#1296Closesapache#1328Closesapache#1342Closesapache#1353Closesapache#1370Closesapache#1376Closesapache#1391Closesapache#1395Closesapache#1399Closesapache#1406Closesapache#1410Closesapache#1422Closesapache#1427Closesapache#1443Closesapache#1462Closesapache#1468Closesapache#1483Closesapache#1506Closesapache#1509Closesapache#1515Closesapache#1520Closesapache#1521Closesapache#1525Closesapache#1527Closesapache#1544Closesapache#1550Closesapache#1566Closesapache#1569Closesapache#1570Closesapache#1575Closesapache#1580Closesapache#1584Closesapache#1591Closesapache#1600Closesapache#1611Closesapache#1613Closesapache#1639Closesapache#1703Closesapache#1711Closesapache#1719Closesapache#1737Closesapache#1760Closesapache#1767Closesapache#1768Closesapache#1785Closesapache#1799Closesapache#1822Closesapache#1824Closesapache#1844Closesapache#1874Closesapache#1918Closesapache#1928Closesapache#1937Closesapache#1942Closesapache#1951Closesapache#1957Closesapache#1963Closesapache#1964Closesapache#1965Closesapache#1967Closesapache#1968Closesapache#1971Closesapache#1985Closesapache#1986Closesapache#1998Closesapache#2031Closesapache#2032Closesapache#2071Closesapache#2076Closesapache#2108Closesapache#2119Closesapache#2128Closesapache#2142Closesapache#2174Closesapache#2206Closesapache#2297Closesapache#2322Closesapache#2332Closesapache#2341Closesapache#2377Closesapache#2414Closesapache#2469
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I have a case that the SolrCloud in my organization is protected by SSL + Basic Auth, so I need to provide customized SolrClient instance to SolrUpdateBolt. Likewise, the RestJsonSchemaBuilder cannot access Schema API without auth.
In addition, for "near real-time search", soft commit is preferred, so it is better to expose an interface for user to customize commit operation.
I think those features will also be useful for other people who needs to use custom SolrClient implementation and control the detail of commit operation. So, finally, I plan an enhancement for storm-solr: