magibney commented on code in PR #889: URL: https://github.com/apache/solr/pull/889#discussion_r887942257
########## solr/solr-ref-guide/modules/indexing-guide/pages/indexing-with-update-handlers.adoc: ########## @@ -18,15 +18,15 @@ // under the License. Update handlers are request handlers designed to add, delete and update documents to the index. -In addition to having plugins for importing rich documents xref:indexing-with-tika.adoc[], Solr natively supports indexing structured documents in XML, CSV, and JSON. +In addition to having plugins for importing rich documents (see xref:indexing-with-tika.adoc[]), Solr natively supports indexing structured documents in XML, CSV, and JSON. The recommended way to configure and use request handlers is with path based names that map to paths in the request URL. However, request handlers can also be specified with the `qt` (query type) parameter if the xref:configuration-guide:requestdispatcher.adoc[`requestDispatcher`] is appropriately configured. It is possible to access the same handler using more than one name, which can be useful if you wish to specify different sets of default options. A single unified update request handler supports XML, CSV, JSON, and javabin update requests, delegating to the appropriate `ContentStreamLoader` based on the `Content-Type` of the xref:content-streams.adoc[ContentStream]. -If you need to pre-process documents after they are loaded but before they are indexed (or even checked against the schema), +If you need to preprocess documents after they are loaded but before they are indexed (or even checked against the schema), Review Comment: This change seems out-of-scope; assuming it makes language more consistent, I guess it's fine to leave it in here. Can you confirm that "preprocess" is more commonly used? Both seem fine to me, though good to standardize on one. ########## solr/solr-ref-guide/modules/configuration-guide/pages/solr-modules.adoc: ########## @@ -17,32 +17,34 @@ // specific language governing permissions and limitations // under the License. -Solr modules are addon Solr plugins that are not part of solr-core, but officially maintained +Solr Modules are addon Solr plugins that are not part of solr-core, but officially maintained by the Solr project. They provide well-defined features such as the "extracting" module which lets users index rich text documents with Apache Tika. A single module can contain multiple Plugins. Modules were earlier known as "contribs". Review Comment: Is it worth explicitly mentioning somewhere in here that Solr Modules are not the same as Java (JPMS) modules? If so, adding that as part of this PR would make sense. ########## solr/solr-ref-guide/modules/configuration-guide/pages/solr-modules.adoc: ########## @@ -17,32 +17,34 @@ // specific language governing permissions and limitations // under the License. -Solr modules are addon Solr plugins that are not part of solr-core, but officially maintained +Solr Modules are addon Solr plugins that are not part of solr-core, but officially maintained by the Solr project. They provide well-defined features such as the "extracting" module which lets users index rich text documents with Apache Tika. A single module can contain multiple Plugins. Modules were earlier known as "contribs". -Each module produces a separate `.jar` file in the build, and additional dependencies required by -each module are also packaged with the module. This helps keep the main core of Solr small and lean. +Each module produces a separate `.jar` file in the build, and any additional dependencies required +are also packaged with the module and therefore included for you. This helps keep the main core of Solr +small and lean. == Installing a module The easiest way to enable a module is to list the modules you intend to use either in the system property `solr.modules` or in the environment variable `SOLR_MODULES` (e.g. in `solr.in.sh` or `solr.in.cmd`). You can also add a `<str name="modules">` tag to xref:configuration-guide:configuring-solr-xml.adoc[solr.xml]. The expected value is a comma separated list -of module names, e.g. `SOLR_MODULES=extracting,ltr`. This way of adding modules will add -them to the shared class loader, making them available for every collection in Solr. +of module names, e.g. `SOLR_MODULES=extracting,ltr`. This will add +them to the shared class loader, making them available to every collection in Solr. Review Comment: Just a sanity-check on what "add them to the shared class loader" means exactly. Particularly in the context of SOLR-16219, it's clear that at a minimum it would be more accurate to say "add them to to _a_ shared class loader" -- or from the user's perspective, perhaps it would suffice to simply say "This will make the functionality of configured Modules available to every collection" (perhaps really "core" rather than "collection"; strictly speaking since this is a node-level config, the effect of enabling this would be at the "core" level? idk.) ########## solr/solr-ref-guide/modules/configuration-guide/pages/libs.adoc: ########## @@ -29,19 +29,18 @@ If there is overlap or inter-dependencies between libraries, then pay attention There are several special places you can place Solr plugin `.jar` files: * `<solr_home>/lib/`: The `.jar` files placed here are available to all Solr cores running on the node, and to node level plugins referenced in `solr.xml` -- so basically everything. -This directory is not present by default so create it. +This directory is not present by default so you willl need to create it. Review Comment: typo "willl" => "will" ########## solr/solr-ref-guide/modules/configuration-guide/pages/libs.adoc: ########## @@ -29,19 +29,18 @@ If there is overlap or inter-dependencies between libraries, then pay attention There are several special places you can place Solr plugin `.jar` files: * `<solr_home>/lib/`: The `.jar` files placed here are available to all Solr cores running on the node, and to node level plugins referenced in `solr.xml` -- so basically everything. -This directory is not present by default so create it. +This directory is not present by default so you willl need to create it. Review Comment: typo "willl" => "will" ########## solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-9.adoc: ########## @@ -111,8 +111,9 @@ This may require some more disk space for logs than was the case in version 8.x. changed package name to `org.apache.solr.security.jwt`, but can still be loaded as shortform `class="solr.JWTAuthPlugin"`. * Dependency updates - A lot of dependency updates removes several security issues from dependencies, and thus make Solr more secure. * The allow-list defining allowed URLs for the `shards` parameter is not in the `shardHandler` configuration anymore. It is defined by the `allowUrls` top-level property of the `solr.xml` file. For more information, see xref:configuration-guide:configuring-solr-xml.adoc#allow-urls[Format of solr.allowUrls] documentation. -* To improve security, `StatelessScriptUpdateProcessorFactory` has been renamed as `ScriptUpdateProcessorFactory` and moved to `modules/scripting` package instead of shipping as part of Solr core. -* To improve security, `XSLTResponseWriter` has been moved to `modules/scripting` package instead of shipping as part of Solr core. +* To improve security, `StatelessScriptUpdateProcessorFactory` has been renamed as `ScriptUpdateProcessorFactory` and moved to `scripting` module instead of shipping as part of Solr core. Users need to add the module `scripting` to classpath. +* To improve security, `XSLTResponseWriter` has been moved to `scripting` module instead of shipping as part of Solr core. Users need to add the module `scripting` to classpath. + Review Comment: for consistency with the rest of the approach in this PR, prefer "must be enabled" language (as opposed to specifically referring to adding to the classpath)? Probably not worth changing if it was already there, but this "classpath" language is introduced fresh with this PR. ########## solr/solr-ref-guide/modules/configuration-guide/pages/solr-modules.adoc: ########## @@ -17,32 +17,34 @@ // specific language governing permissions and limitations // under the License. -Solr modules are addon Solr plugins that are not part of solr-core, but officially maintained +Solr Modules are addon Solr plugins that are not part of solr-core, but officially maintained by the Solr project. They provide well-defined features such as the "extracting" module which lets users index rich text documents with Apache Tika. A single module can contain multiple Plugins. Modules were earlier known as "contribs". -Each module produces a separate `.jar` file in the build, and additional dependencies required by -each module are also packaged with the module. This helps keep the main core of Solr small and lean. +Each module produces a separate `.jar` file in the build, and any additional dependencies required +are also packaged with the module and therefore included for you. This helps keep the main core of Solr +small and lean. == Installing a module The easiest way to enable a module is to list the modules you intend to use either in the system property `solr.modules` or in the environment variable `SOLR_MODULES` (e.g. in `solr.in.sh` or `solr.in.cmd`). You can also add a `<str name="modules">` tag to xref:configuration-guide:configuring-solr-xml.adoc[solr.xml]. The expected value is a comma separated list -of module names, e.g. `SOLR_MODULES=extracting,ltr`. This way of adding modules will add -them to the shared class loader, making them available for every collection in Solr. +of module names, e.g. `SOLR_MODULES=extracting,ltr`. This will add +them to the shared class loader, making them available to every collection in Solr. -You can also specify the modules when using the Solr CLI to start Solr: +You can also specify the modules to include when using the Solr CLI to start Solr: [source,bash] ---- bin/solr start -e techproducts -Dsolr.modules=scripting ---- -If you only wish to enable a module for a single collection, you may add `<lib>` tags to `solrconfig.xml` -as explained in xref:configuration-guide:libs.adoc[Lib Directories]. +NOTE: If you only wish to enable a module for a single collection, you may add `<lib>` tags to `solrconfig.xml` +as explained in xref:configuration-guide:libs.adoc[Lib Directories]. This technically isn't considered enabling +a module, as you are instead just loading all the jars in the module's `lib/` folder, thus enabling all that module' plugins. Review Comment: capitalize "Module" for consistency with the approach this PR is taking. >This technically isn't considered enabling a module, as you are instead just loading all the jars in the module's `lib/` folder, thus enabling all that module' plugins. I'm not sure I see the purpose of this addition. Functionally it's identical to enabling a module, with the only distinction being that the `<lib>` approach is scoped to a single collection -- but that's already mentioned above. ########## solr/solr-ref-guide/modules/query-guide/pages/sql-query.adoc: ########## @@ -21,11 +21,16 @@ // specific language governing permissions and limitations // under the License. -The Solr SQL module brings the power of SQL querying to Solr. The SQL module needs to be xref:configuration-guide:solr-modules.adoc[installed] to use. - -The SQL interface seamlessly combines SQL with Solr's full-text search capabilities. +The Solr SQL module brings the power of SQL querying to Solr by seamlessly combining Review Comment: capitalize "Module" for consistency with the approach taken in this PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
