You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "ilariapet (via GitHub)" <gi...@apache.org> on 2023/03/23 11:08:45 UTC
[GitHub] [solr] ilariapet opened a new pull request, #1487: SOLR-15493: Throw an error message if the feature store doesn't exist
ilariapet opened a new pull request, #1487:
URL: https://github.com/apache/solr/pull/1487
https://issues.apache.org/jira/browse/SOLR-15493
https://issues.apache.org/jira/browse/SOLR-15494
<!--
_(If you are a project committer then you may remove some/all of the following template.)_
Before creating a pull request, please file an issue in the ASF Jira system for Solr:
* https://issues.apache.org/jira/projects/SOLR
For something minor (i.e. that wouldn't be worth putting in release notes), you can skip JIRA.
To create a Jira issue, you will need to create an account there first.
The title of the PR should reference the Jira issue number in the form:
* SOLR-####: <short description of problem or changes>
SOLR must be fully capitalized. A short description helps people scanning pull requests for items they can work on.
Properly referencing the issue in the title ensures that Jira is correctly updated with code review comments and commits. -->
# Description
When a wrong/non-existent feature store name is passed to the query for feature extraction, an empty list is returned;
it also happens when inspecting the contents of a non-existent feature store. It would be helpful if a message was returned indicating that the feature store I am calling is not present.
Also, this non-existent (and empty) feature store is inappropriately added to the feature store list and only disappears after the collection is reloaded.
This contribution wants to add the ability to throw an exception when a non-existent feature store name is passed and thus wants to prevent it from being inappropriately added to the feature store list.
# Solution
A check for a non-existent feature store was added in the doGet method of the _ManagedFeatureStore.java_ class, throwing a Solr exception.
Also, a getter method of the "stores" variable was added in the _ManagedFeatureStore.java_ class and used to throw the exception during the feature extraction (logging phase) if the passed feature store is missing from the store list.
In this phase, there is the need to differentiate between the two cases where no feature store name is passed (i.e., it is null):
If a reranking query (qr) is not passed, it means the model name is not passed either, so the only feature store that could be used is the default feature store (which should not be empty, otherwise an error is thrown).
On the other hand, if a reranking query is passed, it means that a model name has been specified, so a temporarily empty feature store is created to later add features in the implTransform() method, using the methods of the FeatureLogger class.
# Tests
Here are the tests that have been added to test the new capability.
To verify that an Exception is thrown when a non-existent feature store is passed to the query (with fl parameter) 3 tests have been added in _solr/modules/ltr/src/test/org/apache/solr/ltr/feature/TestFeatureLogging.java_:
- _featureExtraction_NonExistentFeatureStore_shouldThrowException_
- _featureExtraction_NonExistentFeatureStoreReRanking_shouldThrowException_
- _featureExtraction_NoFeatureAndModelStore_shouldThrowException_
To verify that an Exception is thrown when trying to get a missing feature store and that this feature store is not inappropriately added to the feature store list, some tests in solr/modules/ltr/src/test/org/apache/solr/ltr/store/rest/TestModelManagerPersistence.java have been modified.
# Checklist
Please review the following and check all that apply:
- [X] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [X] I have created a Jira issue and added the issue ID to my pull request title.
- [X] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [X] I have developed this patch against the `main` branch.
- [X] I have run `./gradlew check`.
- [X] I have added tests for my changes.
- [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] ilariapet commented on a diff in pull request #1487: SOLR-15493: Throw an error message if the feature store doesn't exist
Posted by "ilariapet (via GitHub)" <gi...@apache.org>.
ilariapet commented on code in PR #1487:
URL: https://github.com/apache/solr/pull/1487#discussion_r1169863438
##########
solr/modules/ltr/src/java/org/apache/solr/ltr/response/transform/LTRFeatureLoggerTransformerFactory.java:
##########
@@ -241,10 +242,31 @@ public void setContext(ResultContext context) {
*
* @param transformerFeatureStore the explicit transformer feature store
*/
- private LoggingModel createLoggingModel(String transformerFeatureStore) {
- final ManagedFeatureStore fr = ManagedFeatureStore.getManagedFeatureStore(req.getCore());
+ private LoggingModel createLoggingModel(
+ String transformerFeatureStore, Boolean docsWereNotReranked) {
+ final ManagedFeatureStore featureStore =
+ ManagedFeatureStore.getManagedFeatureStore(req.getCore());
+
+ // check for non-existent feature store name
+ if (transformerFeatureStore != null) {
+ if (!featureStore.getStores().containsKey(transformerFeatureStore)) {
Review Comment:
If I have understood correctly your doubt...
I added two checks because _doGet_ method is used when you want to get a feature store, for example:
`GET http://localhost:8983/solr/books/schema/feature-store/nonExistentFeatureStore`
While _createLoggingModel()_ method is used when you want to extract features, for example:
`GET http://localhost:8983/solr/books/query?q=test&fl=id,[features store=nonExistentFeatureStore]
`
Here I also handled the cases when the store name is not passed (i.e., is null [features]) and when the rerank query is present.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] alessandrobenedetti merged pull request #1487: SOLR-15493: Throw an error message if the feature store doesn't exist
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti merged PR #1487:
URL: https://github.com/apache/solr/pull/1487
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] alessandrobenedetti commented on a diff in pull request #1487: SOLR-15493: Throw an error message if the feature store doesn't exist
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #1487:
URL: https://github.com/apache/solr/pull/1487#discussion_r1169868929
##########
solr/CHANGES.txt:
##########
@@ -85,7 +85,9 @@ Improvements
`GET /api/node/logging/messages`. And the threshold for the log "listener" can be modified with
`PUT /api/node/logging/messages/threshold`. (Jason Gerlowski, Calvince Otieno)
-* SOLR-16504: Convert CLI tools to use Jetty HTTP 2 client. (Bence Szabo via Eric Pugh)
+* SOLR-16504: Convert CLI tools to use Jetty HTTP 2 client. (Bence Szabo via Eric Pugh)
Review Comment:
line 88 should result unchanged
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] alessandrobenedetti commented on a diff in pull request #1487: SOLR-15493: Throw an error message if the feature store doesn't exist
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #1487:
URL: https://github.com/apache/solr/pull/1487#discussion_r1169843711
##########
solr/modules/ltr/src/java/org/apache/solr/ltr/response/transform/LTRFeatureLoggerTransformerFactory.java:
##########
@@ -241,10 +242,31 @@ public void setContext(ResultContext context) {
*
* @param transformerFeatureStore the explicit transformer feature store
*/
- private LoggingModel createLoggingModel(String transformerFeatureStore) {
- final ManagedFeatureStore fr = ManagedFeatureStore.getManagedFeatureStore(req.getCore());
+ private LoggingModel createLoggingModel(
+ String transformerFeatureStore, Boolean docsWereNotReranked) {
+ final ManagedFeatureStore featureStore =
+ ManagedFeatureStore.getManagedFeatureStore(req.getCore());
+
+ // check for non-existent feature store name
+ if (transformerFeatureStore != null) {
+ if (!featureStore.getStores().containsKey(transformerFeatureStore)) {
Review Comment:
the formatting is terrible but you should get the idea
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] ilariapet commented on a diff in pull request #1487: SOLR-15493: Throw an error message if the feature store doesn't exist
Posted by "ilariapet (via GitHub)" <gi...@apache.org>.
ilariapet commented on code in PR #1487:
URL: https://github.com/apache/solr/pull/1487#discussion_r1169918755
##########
solr/CHANGES.txt:
##########
@@ -85,7 +85,9 @@ Improvements
`GET /api/node/logging/messages`. And the threshold for the log "listener" can be modified with
`PUT /api/node/logging/messages/threshold`. (Jason Gerlowski, Calvince Otieno)
-* SOLR-16504: Convert CLI tools to use Jetty HTTP 2 client. (Bence Szabo via Eric Pugh)
+* SOLR-16504: Convert CLI tools to use Jetty HTTP 2 client. (Bence Szabo via Eric Pugh)
Review Comment:
oh ok... I have also seen that this file was recently modified at the same point and our fork doesn't contain the change
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] alessandrobenedetti commented on a diff in pull request #1487: SOLR-15493: Throw an error message if the feature store doesn't exist
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #1487:
URL: https://github.com/apache/solr/pull/1487#discussion_r1169872799
##########
solr/modules/ltr/src/java/org/apache/solr/ltr/response/transform/LTRFeatureLoggerTransformerFactory.java:
##########
@@ -241,10 +242,31 @@ public void setContext(ResultContext context) {
*
* @param transformerFeatureStore the explicit transformer feature store
*/
- private LoggingModel createLoggingModel(String transformerFeatureStore) {
- final ManagedFeatureStore fr = ManagedFeatureStore.getManagedFeatureStore(req.getCore());
+ private LoggingModel createLoggingModel(
+ String transformerFeatureStore, Boolean docsWereNotReranked) {
+ final ManagedFeatureStore featureStores =
+ ManagedFeatureStore.getManagedFeatureStore(req.getCore());
+
+ // check for non-existent feature store name
+ if (transformerFeatureStore != null) {
+ if (!featureStores.getStores().containsKey(transformerFeatureStore)) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "Missing feature store: " + transformerFeatureStore);
+ }
+ }
Review Comment:
let me rephrase it:
what happens if you entirely remove this block
`// check for non-existent feature store name
if (transformerFeatureStore != null) {
if (!featureStores.getStores().containsKey(transformerFeatureStore)) {
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST,
"Missing feature store: " + transformerFeatureStore);
}
}`
Shouldn't 'final FeatureStore store = featureStores.getFeatureStore(transformerFeatureStore);' raise the same exception?
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] ilariapet commented on a diff in pull request #1487: SOLR-15493: Throw an error message if the feature store doesn't exist
Posted by "ilariapet (via GitHub)" <gi...@apache.org>.
ilariapet commented on code in PR #1487:
URL: https://github.com/apache/solr/pull/1487#discussion_r1169643395
##########
solr/modules/ltr/src/java/org/apache/solr/ltr/response/transform/LTRFeatureLoggerTransformerFactory.java:
##########
@@ -241,10 +242,31 @@ public void setContext(ResultContext context) {
*
* @param transformerFeatureStore the explicit transformer feature store
*/
- private LoggingModel createLoggingModel(String transformerFeatureStore) {
- final ManagedFeatureStore fr = ManagedFeatureStore.getManagedFeatureStore(req.getCore());
+ private LoggingModel createLoggingModel(
+ String transformerFeatureStore, Boolean docsWereNotReranked) {
+ final ManagedFeatureStore featureStore =
+ ManagedFeatureStore.getManagedFeatureStore(req.getCore());
+
+ // check for non-existent feature store name
+ if (transformerFeatureStore != null) {
+ if (!featureStore.getStores().containsKey(transformerFeatureStore)) {
Review Comment:
When a nonexistent feature store is passed with _"final FeatureStore store = featureStore.getFeatureStore(transformerFeatureStore);"_ no error is received and the store is improperly added (empty) to the list of feature stores. In fact, the getFeatureStore() method is also used for the 'put' phase.
So a check was added to get the error and avoid creating it.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] alessandrobenedetti commented on a diff in pull request #1487: SOLR-15493: Throw an error message if the feature store doesn't exist
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #1487:
URL: https://github.com/apache/solr/pull/1487#discussion_r1168810162
##########
solr/modules/ltr/src/test/org/apache/solr/ltr/feature/TestExternalFeatures.java:
##########
@@ -200,6 +200,16 @@ public void featureExtraction_valueFeatureRequired_shouldThrowException() throws
"/error/msg=='Exception from createWeight for ValueFeature [name=popularity, params={value=${myPop}, required=true}] ValueFeatureWeight requires efi parameter that was not passed in request.'");
}
+ @Test
+ public void featureExtraction_wrongFeatureStore_shouldThrowException() throws Exception {
+ final SolrQuery query = new SolrQuery();
+ query.setQuery("*:*");
+ query.add("rows", "1");
+ query.add("fl", "fvalias:[fv store=featureStoreWrong]");
Review Comment:
should this be 'nonExistent' rather than 'wrong' ?
##########
solr/modules/ltr/src/java/org/apache/solr/ltr/response/transform/LTRFeatureLoggerTransformerFactory.java:
##########
@@ -241,10 +242,31 @@ public void setContext(ResultContext context) {
*
* @param transformerFeatureStore the explicit transformer feature store
*/
- private LoggingModel createLoggingModel(String transformerFeatureStore) {
- final ManagedFeatureStore fr = ManagedFeatureStore.getManagedFeatureStore(req.getCore());
+ private LoggingModel createLoggingModel(
+ String transformerFeatureStore, Boolean docsWereNotReranked) {
+ final ManagedFeatureStore featureStore =
+ ManagedFeatureStore.getManagedFeatureStore(req.getCore());
+
+ // check for non-existent feature store name
+ if (transformerFeatureStore != null) {
+ if (!featureStore.getStores().containsKey(transformerFeatureStore)) {
Review Comment:
do we need this? isn't this already in the 'final FeatureStore store = featureStore.getFeatureStore(transformerFeatureStore);' ?
Do we even need 'featureStore.getStores()' ?
##########
solr/modules/ltr/src/test/org/apache/solr/ltr/feature/TestFeatureLogging.java:
##########
@@ -125,6 +126,11 @@ public void testDefaultStoreFeatureExtraction() throws Exception {
+ FeatureLoggerTestUtils.toFeatureVector("defaultf1", "1.0")
+ "'}");
+ // Wrong store specified, should throw exception
+ query.remove("fl");
+ query.add("fl", "fv:[fv store=storeWrong]");
Review Comment:
should it be 'nonExistent' ?
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] alessandrobenedetti commented on a diff in pull request #1487: SOLR-15493: Throw an error message if the feature store doesn't exist
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #1487:
URL: https://github.com/apache/solr/pull/1487#discussion_r1169839350
##########
solr/modules/ltr/src/java/org/apache/solr/ltr/response/transform/LTRFeatureLoggerTransformerFactory.java:
##########
@@ -241,10 +242,31 @@ public void setContext(ResultContext context) {
*
* @param transformerFeatureStore the explicit transformer feature store
*/
- private LoggingModel createLoggingModel(String transformerFeatureStore) {
- final ManagedFeatureStore fr = ManagedFeatureStore.getManagedFeatureStore(req.getCore());
+ private LoggingModel createLoggingModel(
+ String transformerFeatureStore, Boolean docsWereNotReranked) {
+ final ManagedFeatureStore featureStore =
+ ManagedFeatureStore.getManagedFeatureStore(req.getCore());
+
+ // check for non-existent feature store name
+ if (transformerFeatureStore != null) {
+ if (!featureStore.getStores().containsKey(transformerFeatureStore)) {
Review Comment:
mmmm what is the difference with:
`public synchronized FeatureStore getFeatureStore(String name) {
if (name == null) {
name = FeatureStore.DEFAULT_FEATURE_STORE_NAME;
if (childId == null) {
response.add(FEATURE_STORE_JSON_FIELD, stores.keySet());
} else {
// check for non-existent feature store name
if (!stores.containsKey(childId)) {
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST, "Missing feature store: " + childId);
}` ?
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] ilariapet commented on a diff in pull request #1487: SOLR-15493: Throw an error message if the feature store doesn't exist
Posted by "ilariapet (via GitHub)" <gi...@apache.org>.
ilariapet commented on code in PR #1487:
URL: https://github.com/apache/solr/pull/1487#discussion_r1169916567
##########
solr/modules/ltr/src/java/org/apache/solr/ltr/response/transform/LTRFeatureLoggerTransformerFactory.java:
##########
@@ -241,10 +242,31 @@ public void setContext(ResultContext context) {
*
* @param transformerFeatureStore the explicit transformer feature store
*/
- private LoggingModel createLoggingModel(String transformerFeatureStore) {
- final ManagedFeatureStore fr = ManagedFeatureStore.getManagedFeatureStore(req.getCore());
+ private LoggingModel createLoggingModel(
+ String transformerFeatureStore, Boolean docsWereNotReranked) {
+ final ManagedFeatureStore featureStores =
+ ManagedFeatureStore.getManagedFeatureStore(req.getCore());
+
+ // check for non-existent feature store name
+ if (transformerFeatureStore != null) {
+ if (!featureStores.getStores().containsKey(transformerFeatureStore)) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "Missing feature store: " + transformerFeatureStore);
+ }
+ }
Review Comment:
I added the exception in the _doGet_ method, so _getFeatureStore()_ remains unchanged and doesn't raise any exception as it is also used to put new feature stores.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] alessandrobenedetti commented on a diff in pull request #1487: SOLR-15493: Throw an error message if the feature store doesn't exist
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #1487:
URL: https://github.com/apache/solr/pull/1487#discussion_r1169839350
##########
solr/modules/ltr/src/java/org/apache/solr/ltr/response/transform/LTRFeatureLoggerTransformerFactory.java:
##########
@@ -241,10 +242,31 @@ public void setContext(ResultContext context) {
*
* @param transformerFeatureStore the explicit transformer feature store
*/
- private LoggingModel createLoggingModel(String transformerFeatureStore) {
- final ManagedFeatureStore fr = ManagedFeatureStore.getManagedFeatureStore(req.getCore());
+ private LoggingModel createLoggingModel(
+ String transformerFeatureStore, Boolean docsWereNotReranked) {
+ final ManagedFeatureStore featureStore =
+ ManagedFeatureStore.getManagedFeatureStore(req.getCore());
+
+ // check for non-existent feature store name
+ if (transformerFeatureStore != null) {
+ if (!featureStore.getStores().containsKey(transformerFeatureStore)) {
Review Comment:
mmmm what is the difference with:
`public synchronized FeatureStore getFeatureStore(String name) {
if (name == null) {
name = FeatureStore.DEFAULT_FEATURE_STORE_NAME;
@@ -160,11 +164,12 @@ public void doGet(BaseSolrResource endpoint, String childId) {
if (childId == null) {
response.add(FEATURE_STORE_JSON_FIELD, stores.keySet());
} else {
// check for non-existent feature store name
if (!stores.containsKey(childId)) {
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST, "Missing feature store: " + childId);
}` ?
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] alessandrobenedetti commented on a diff in pull request #1487: SOLR-15493: Throw an error message if the feature store doesn't exist
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #1487:
URL: https://github.com/apache/solr/pull/1487#discussion_r1169886237
##########
solr/modules/ltr/src/test/org/apache/solr/ltr/feature/TestFeatureLogging.java:
##########
@@ -152,6 +152,74 @@ public void testDefaultStoreFeatureExtraction() throws Exception {
+ "'}");
}
+ @Test
+ public void featureExtraction_NonExistentFeatureStore_shouldThrowException() throws Exception {
Review Comment:
after the _, lowercase 'nonExistent...'
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] alessandrobenedetti commented on a diff in pull request #1487: SOLR-15493: Throw an error message if the feature store doesn't exist
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #1487:
URL: https://github.com/apache/solr/pull/1487#discussion_r1169981475
##########
solr/modules/ltr/src/java/org/apache/solr/ltr/response/transform/LTRFeatureLoggerTransformerFactory.java:
##########
@@ -241,10 +242,31 @@ public void setContext(ResultContext context) {
*
* @param transformerFeatureStore the explicit transformer feature store
*/
- private LoggingModel createLoggingModel(String transformerFeatureStore) {
- final ManagedFeatureStore fr = ManagedFeatureStore.getManagedFeatureStore(req.getCore());
+ private LoggingModel createLoggingModel(
+ String transformerFeatureStore, Boolean docsWereNotReranked) {
+ final ManagedFeatureStore featureStore =
+ ManagedFeatureStore.getManagedFeatureStore(req.getCore());
+
+ // check for non-existent feature store name
+ if (transformerFeatureStore != null) {
+ if (!featureStore.getStores().containsKey(transformerFeatureStore)) {
Review Comment:
my bad, resolving the comment!
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org