SOLR-11617 rename alias metadata as alias properties #335
Conversation
| @@ -35,7 +35,7 @@ | |||
|
|
|||
| public class ModifyAliasCmd implements Cmd { | |||
There was a problem hiding this comment.
Rename to "SetAliasPropCmd"?
| "&wt=xml" + | ||
| "&name=" + aliasName + | ||
| "&metadata.foo=baz" + | ||
| "&metadata.bar=bam"); | ||
| "&properties.foo=baz" + |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
| @@ -750,36 +750,38 @@ http://localhost:8983/solr/admin/collections?action=LISTALIASES&wt=xml | |||
| ---- | |||
|
|
|||
| [[modifyalias]] | |||
There was a problem hiding this comment.
"modifyalias" here is now incorrect; should be "ALIASPROP". This would affect some of the examples below too.
| @@ -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); | |||
There was a problem hiding this comment.
it'd be nice to also rename aliasMetada to aliasProperties
|
Added another commit addressing comments |
|
https://issues.apache.org/jira/browse/SOLR-11617 was released in Solr 7.3, so this PR can be closed. |
Renamed the usage in the API and made the code match where I could to avoid confusion in the future.