You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by nsoft <gi...@git.apache.org> on 2018/03/10 20:42:27 UTC
[GitHub] lucene-solr pull request #335: SOLR-11617 rename alias metadata as alias pro...
GitHub user nsoft opened a pull request:
https://github.com/apache/lucene-solr/pull/335
SOLR-11617 rename alias metadata as alias properties
Renamed the usage in the API and made the code match where I could to avoid confusion in the future.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nsoft/lucene-solr SOLR-11617-renames
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/lucene-solr/pull/335.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #335
----
commit 609e13514cf489e5cb955b3b7745ce2b243bdc20
Author: Gus Heck <gu...@...>
Date: 2018-03-10T17:54:15Z
rename alias metadata as alias properties
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #335: SOLR-11617 rename alias metadata as alias pro...
Posted by fsparv <gi...@git.apache.org>.
Github user fsparv commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/335#discussion_r173638701
--- Diff: solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java ---
@@ -238,51 +239,52 @@ public void testModifyMetadataV2() throws Exception {
checkFooAndBarMeta(aliasName, zkStateReader);
}
- public void testModifyMetadataV1() throws Exception {
+ @Test
+ public void testModifyPropertiesV1() throws Exception {
// note we don't use TZ in this test, thus it's UTC
final String aliasName = getTestName();
ZkStateReader zkStateReader = createColectionsAndAlias(aliasName);
final String baseUrl = cluster.getRandomJetty(random()).getBaseUrl().toString();
- HttpGet get = new HttpGet(baseUrl + "/admin/collections?action=MODIFYALIAS" +
+ HttpGet get = new HttpGet(baseUrl + "/admin/collections?action=ALIASPROP" +
"&wt=xml" +
"&name=" + aliasName +
- "&metadata.foo=baz" +
- "&metadata.bar=bam");
+ "&properties.foo=baz" +
--- End diff --
I tend to like dot notation to be nounish things. A set of things collectively know as "properties" with member element ".foo" rather than an adjective "property" describing ".foo"
"properties" : {
"foo": "baz",
"bar": "bam"
}
vs
"property" : {
"foo": "baz",
"bar": "bam"
}
The v1 api flattening should match what we do for v2 I think?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #335: SOLR-11617 rename alias metadata as alias pro...
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/335#discussion_r173637222
--- Diff: solr/core/src/java/org/apache/solr/cloud/api/collections/ModifyAliasCmd.java ---
@@ -35,7 +35,7 @@
public class ModifyAliasCmd implements Cmd {
--- End diff --
rename class?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #335: SOLR-11617 rename alias metadata as alias pro...
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/335#discussion_r173637854
--- Diff: solr/core/src/java/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessor.java ---
@@ -124,7 +124,7 @@ protected TimeRoutedAliasUpdateProcessor(SolrQueryRequest req, SolrQueryResponse
cmdDistrib = new SolrCmdDistributor(cc.getUpdateShardHandler());
collHandler = cc.getCollectionsHandler();
- final Map<String, String> aliasMetadata = zkController.getZkStateReader().getAliases().getCollectionAliasMetadata(aliasName);
+ final Map<String, String> aliasMetadata = zkController.getZkStateReader().getAliases().getCollectionAliasProperties(aliasName);
--- End diff --
it'd be nice to also rename aliasMetada to aliasProperties
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #335: SOLR-11617 rename alias metadata as alias properties
Posted by fsparv <gi...@git.apache.org>.
Github user fsparv commented on the issue:
https://github.com/apache/lucene-solr/pull/335
Added another commit addressing comments
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #335: SOLR-11617 rename alias metadata as alias pro...
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/335#discussion_r173637762
--- Diff: solr/solr-ref-guide/src/collections-api.adoc ---
@@ -750,36 +750,38 @@ http://localhost:8983/solr/admin/collections?action=LISTALIASES&wt=xml
----
[[modifyalias]]
--- End diff --
"modifyalias" here is now incorrect; should be "ALIASPROP". This would affect some of the examples below too.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #335: SOLR-11617 rename alias metadata as alias pro...
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/335#discussion_r173641664
--- Diff: solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java ---
@@ -238,51 +239,52 @@ public void testModifyMetadataV2() throws Exception {
checkFooAndBarMeta(aliasName, zkStateReader);
}
- public void testModifyMetadataV1() throws Exception {
+ @Test
+ public void testModifyPropertiesV1() throws Exception {
// note we don't use TZ in this test, thus it's UTC
final String aliasName = getTestName();
ZkStateReader zkStateReader = createColectionsAndAlias(aliasName);
final String baseUrl = cluster.getRandomJetty(random()).getBaseUrl().toString();
- HttpGet get = new HttpGet(baseUrl + "/admin/collections?action=MODIFYALIAS" +
+ HttpGet get = new HttpGet(baseUrl + "/admin/collections?action=ALIASPROP" +
"&wt=xml" +
"&name=" + aliasName +
- "&metadata.foo=baz" +
- "&metadata.bar=bam");
+ "&properties.foo=baz" +
--- End diff --
What you have for V2 (a JSON API) — “properties” is good. I’m directing my feedback expressly at V1 for “property”, which is a param prefix, not JSON. Again, see the existing APIs like core properties which already set this precedent.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #335: SOLR-11617 rename alias metadata as alias pro...
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/335#discussion_r173637697
--- Diff: solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java ---
@@ -238,51 +239,52 @@ public void testModifyMetadataV2() throws Exception {
checkFooAndBarMeta(aliasName, zkStateReader);
}
- public void testModifyMetadataV1() throws Exception {
+ @Test
+ public void testModifyPropertiesV1() throws Exception {
// note we don't use TZ in this test, thus it's UTC
final String aliasName = getTestName();
ZkStateReader zkStateReader = createColectionsAndAlias(aliasName);
final String baseUrl = cluster.getRandomJetty(random()).getBaseUrl().toString();
- HttpGet get = new HttpGet(baseUrl + "/admin/collections?action=MODIFYALIAS" +
+ HttpGet get = new HttpGet(baseUrl + "/admin/collections?action=ALIASPROP" +
"&wt=xml" +
"&name=" + aliasName +
- "&metadata.foo=baz" +
- "&metadata.bar=bam");
+ "&properties.foo=baz" +
--- End diff --
I think we want the V1 API prefix for this to be "property" since each param occurrence is one property by itself. This is consistent with various other V1 APIs (See CollectionApiMapping.java search for "property"). (My comment here applies to many spots in the diff, not just here of course).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org