You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2023/01/19 23:33:49 UTC

[GitHub] [solr] stillalex opened a new pull request, #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

stillalex opened a new pull request, #1304:
URL: https://github.com/apache/solr/pull/1304

   https://issues.apache.org/jira/browse/SOLR-16618
   
   <!--
   _(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
   
   Please provide a short description of the changes you're making with this pull request.
   
   # Solution
   
   Please provide a short description of the approach taken to implement your solution.
   
   # Tests
   
   Please describe the tests you've developed or run to confirm this patch implements the feature or solves the problem.
   
   # Checklist
   
   Please review the following and check all that apply:
   
   - [ ] 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.
   - [ ] I have created a Jira issue and added the issue ID to my pull request title.
   - [ ] 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)
   - [ ] I have developed this patch against the `main` branch.
   - [ ] I have run `./gradlew check`.
   - [ ] 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] stillalex commented on a diff in pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #1304:
URL: https://github.com/apache/solr/pull/1304#discussion_r1083315721


##########
solr/webapp/web/js/angular/controllers/analysis.js:
##########
@@ -21,33 +21,64 @@ solrAdminApp.controller('AnalysisController',
 
       $scope.refresh = function() {
         Luke.schema({core: $routeParams.core}, function(data) {
-          $scope.fieldsAndTypes = [];
-          for (var field in data.schema.fields) {
-            $scope.fieldsAndTypes.push({
-              group: "Fields",
-              value: "fieldname=" + field,
-              label: field});
-          }
-          for (var type in data.schema.types) {
-            $scope.fieldsAndTypes.push({
-              group: "Types",
-              value: "fieldtype=" + type,
-              label: type});
-          }
+          $scope.fieldsAndTypes = getFieldsAndTypes(data.schema);
           $scope.core = $routeParams.core;
+          $scope.parseQueryString(data.schema);
+          // @todo - set defaultSearchField either to context["analysis.fieldname"] or context["analysis.fieldtype"]

Review Comment:
   I am not exactly sure, left the todo there just in case



-- 
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] stillalex commented on pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #1304:
URL: https://github.com/apache/solr/pull/1304#issuecomment-1400620486

   > Maybe we don't need the default value if we just show the page, but don't submit it to the backend, which then avoids the error message.
   
   I agree with this idea, but the way the current code works, it will trigger the call based on the params existence (I think) so it would not be easy to distinguish 'a back link to this page from schema' vs a 'form submit', not too versed on js code so correct me if this is wrong. dunno, maybe add another param to distinguish?
   


-- 
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] epugh commented on a diff in pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #1304:
URL: https://github.com/apache/solr/pull/1304#discussion_r1083292547


##########
solr/webapp/web/js/angular/controllers/analysis.js:
##########
@@ -137,16 +168,24 @@ solrAdminApp.controller('AnalysisController',
         }
 
         if (fieldOrType == "fieldname") {
-          $location.search("analysis.fieldname", name);
-          $location.search("analysis.fieldtype", null);
+            $location.search("analysis.fieldname", name);

Review Comment:
   I don't love the whitespace change, however I see we have a werid mix of two and four space indentations on if/else throughout this method!   We need some code styling tools for jS ;-)



-- 
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] epugh commented on a diff in pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #1304:
URL: https://github.com/apache/solr/pull/1304#discussion_r1083293202


##########
solr/webapp/web/js/angular/controllers/analysis.js:
##########
@@ -21,33 +21,64 @@ solrAdminApp.controller('AnalysisController',
 
       $scope.refresh = function() {
         Luke.schema({core: $routeParams.core}, function(data) {
-          $scope.fieldsAndTypes = [];
-          for (var field in data.schema.fields) {
-            $scope.fieldsAndTypes.push({
-              group: "Fields",
-              value: "fieldname=" + field,
-              label: field});
-          }
-          for (var type in data.schema.types) {
-            $scope.fieldsAndTypes.push({
-              group: "Types",
-              value: "fieldtype=" + type,
-              label: type});
-          }
+          $scope.fieldsAndTypes = getFieldsAndTypes(data.schema);
           $scope.core = $routeParams.core;
+          $scope.parseQueryString(data.schema);
+          // @todo - set defaultSearchField either to context["analysis.fieldname"] or context["analysis.fieldtype"]
         });
+      };
+      $scope.verbose = true;
 
-        $scope.parseQueryString();
-        // @todo - set defaultSearchField either to context["analysis.fieldname"] or context["analysis.fieldtype"]
+      var getFieldsAndTypes = function(schema) {
+        var aggregatedFields = schema.fields;
+        for (var field in schema.fields) {
+          var copy_dests = schema.fields[field].copyDests;
+          for (var i in copy_dests) {
+            var copy_dest = copy_dests[i];
+            if (!aggregatedFields[copy_dest]) {
+              aggregatedFields[copy_dest] = {};
+            }
+          }
+        }
 
+        var fieldsAndTypes = [];
+        var fields = Object.keys(aggregatedFields).sort();
+        for (var i in fields) {
+          fieldsAndTypes.push({
+            group: "Fields",
+            value: "fieldname=" + fields[i],
+            label: fields[i]
+          });
+        }
+        var dynamic_fields = Object.keys(schema.dynamicFields).sort();

Review Comment:
   camelCase



-- 
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] stillalex commented on pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #1304:
URL: https://github.com/apache/solr/pull/1304#issuecomment-1404022543

   @epugh gentle ping. are we good to merge, or is there something that needs more work?


-- 
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] stillalex commented on pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by GitBox <gi...@apache.org>.
stillalex commented on PR #1304:
URL: https://github.com/apache/solr/pull/1304#issuecomment-1397746600

   @epugh please take a look


-- 
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] stillalex commented on pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #1304:
URL: https://github.com/apache/solr/pull/1304#issuecomment-1400790967

   > Sure! I can buy that. So... leave the todo? It otherwise looks good to merge.
   
   yep, leave the todo. there is only that string matching issue in the combo which I don't know how to approach. open to suggestions or we could leave it for another time.
   
   
   


-- 
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] epugh commented on pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #1304:
URL: https://github.com/apache/solr/pull/1304#issuecomment-1400748206

   > 
   
   Sure!  I can buy that.   So...   leave the todo?   It otherwise looks good to merge.


-- 
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] epugh commented on a diff in pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #1304:
URL: https://github.com/apache/solr/pull/1304#discussion_r1084211406


##########
solr/webapp/web/js/angular/controllers/analysis.js:
##########
@@ -21,33 +21,64 @@ solrAdminApp.controller('AnalysisController',
 
       $scope.refresh = function() {
         Luke.schema({core: $routeParams.core}, function(data) {
-          $scope.fieldsAndTypes = [];
-          for (var field in data.schema.fields) {
-            $scope.fieldsAndTypes.push({
-              group: "Fields",
-              value: "fieldname=" + field,
-              label: field});
-          }
-          for (var type in data.schema.types) {
-            $scope.fieldsAndTypes.push({
-              group: "Types",
-              value: "fieldtype=" + type,
-              label: type});
-          }
+          $scope.fieldsAndTypes = getFieldsAndTypes(data.schema);
           $scope.core = $routeParams.core;
+          $scope.parseQueryString(data.schema);
+          // @todo - set defaultSearchField either to context["analysis.fieldname"] or context["analysis.fieldtype"]

Review Comment:
   Cool, what prompted this?     I guess I am wondering if we need this?   I am playing with the code, and it *appears* to all work ;-)



-- 
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] stillalex commented on pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #1304:
URL: https://github.com/apache/solr/pull/1304#issuecomment-1400705981

   > How hard would it be to change the query to submit when you click the button? Are there other places we link to the schema analysis page and have it auto run, so that requiring a button click would make the user experience worse?
   
   I fear this is going off scope for this PR, this behavior change would deserve its own PR and discussion (and probably someone that actually knows their way around js frontends).
   
   
   


-- 
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] epugh commented on a diff in pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #1304:
URL: https://github.com/apache/solr/pull/1304#discussion_r1083292803


##########
solr/webapp/web/js/angular/controllers/analysis.js:
##########
@@ -21,33 +21,64 @@ solrAdminApp.controller('AnalysisController',
 
       $scope.refresh = function() {
         Luke.schema({core: $routeParams.core}, function(data) {
-          $scope.fieldsAndTypes = [];
-          for (var field in data.schema.fields) {
-            $scope.fieldsAndTypes.push({
-              group: "Fields",
-              value: "fieldname=" + field,
-              label: field});
-          }
-          for (var type in data.schema.types) {
-            $scope.fieldsAndTypes.push({
-              group: "Types",
-              value: "fieldtype=" + type,
-              label: type});
-          }
+          $scope.fieldsAndTypes = getFieldsAndTypes(data.schema);
           $scope.core = $routeParams.core;
+          $scope.parseQueryString(data.schema);
+          // @todo - set defaultSearchField either to context["analysis.fieldname"] or context["analysis.fieldtype"]

Review Comment:
   I don't see defaultSearchField used in the JS or HTML?



-- 
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] stillalex commented on a diff in pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #1304:
URL: https://github.com/apache/solr/pull/1304#discussion_r1083316512


##########
solr/webapp/web/js/angular/controllers/analysis.js:
##########
@@ -137,16 +168,24 @@ solrAdminApp.controller('AnalysisController',
         }
 
         if (fieldOrType == "fieldname") {
-          $location.search("analysis.fieldname", name);
-          $location.search("analysis.fieldtype", null);
+            $location.search("analysis.fieldname", name);

Review Comment:
   yep, fixed the formatting. tried as much as possible to keep the original style



-- 
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] epugh commented on pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #1304:
URL: https://github.com/apache/solr/pull/1304#issuecomment-1404038382

   @magibney and I chatted, and we're going to leave it open till end of the week in case anyone else has concerns about having a potentially longer list of dynamic fields in the drop down ;-).  Sorry, I meant to update here.   If no one has strong concerns, then we'll merge.  


-- 
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] stillalex commented on pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #1304:
URL: https://github.com/apache/solr/pull/1304#issuecomment-1404046037

   makes sense, thanks for the update @epugh!


-- 
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] epugh merged pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh merged PR #1304:
URL: https://github.com/apache/solr/pull/1304


-- 
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] epugh commented on a diff in pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #1304:
URL: https://github.com/apache/solr/pull/1304#discussion_r1083293157


##########
solr/webapp/web/js/angular/controllers/analysis.js:
##########
@@ -21,33 +21,64 @@ solrAdminApp.controller('AnalysisController',
 
       $scope.refresh = function() {
         Luke.schema({core: $routeParams.core}, function(data) {
-          $scope.fieldsAndTypes = [];
-          for (var field in data.schema.fields) {
-            $scope.fieldsAndTypes.push({
-              group: "Fields",
-              value: "fieldname=" + field,
-              label: field});
-          }
-          for (var type in data.schema.types) {
-            $scope.fieldsAndTypes.push({
-              group: "Types",
-              value: "fieldtype=" + type,
-              label: type});
-          }
+          $scope.fieldsAndTypes = getFieldsAndTypes(data.schema);
           $scope.core = $routeParams.core;
+          $scope.parseQueryString(data.schema);
+          // @todo - set defaultSearchField either to context["analysis.fieldname"] or context["analysis.fieldtype"]
         });
+      };
+      $scope.verbose = true;
 
-        $scope.parseQueryString();
-        // @todo - set defaultSearchField either to context["analysis.fieldname"] or context["analysis.fieldtype"]
+      var getFieldsAndTypes = function(schema) {
+        var aggregatedFields = schema.fields;
+        for (var field in schema.fields) {
+          var copy_dests = schema.fields[field].copyDests;
+          for (var i in copy_dests) {
+            var copy_dest = copy_dests[i];

Review Comment:
   camelCase instead of snake_case



-- 
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] epugh commented on a diff in pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #1304:
URL: https://github.com/apache/solr/pull/1304#discussion_r1083292726


##########
solr/webapp/web/js/angular/controllers/analysis.js:
##########
@@ -21,33 +21,64 @@ solrAdminApp.controller('AnalysisController',
 
       $scope.refresh = function() {
         Luke.schema({core: $routeParams.core}, function(data) {
-          $scope.fieldsAndTypes = [];
-          for (var field in data.schema.fields) {
-            $scope.fieldsAndTypes.push({
-              group: "Fields",
-              value: "fieldname=" + field,
-              label: field});
-          }
-          for (var type in data.schema.types) {
-            $scope.fieldsAndTypes.push({
-              group: "Types",
-              value: "fieldtype=" + type,
-              label: type});
-          }
+          $scope.fieldsAndTypes = getFieldsAndTypes(data.schema);
           $scope.core = $routeParams.core;
+          $scope.parseQueryString(data.schema);
+          // @todo - set defaultSearchField either to context["analysis.fieldname"] or context["analysis.fieldtype"]

Review Comment:
   is this still a valid todo?



-- 
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] stillalex commented on pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #1304:
URL: https://github.com/apache/solr/pull/1304#issuecomment-1400615323

   > I see the ? from the bool... I was trying to figure out if there is something we could put there, like "Bool" or something? But regardless, I like how this works...
   
   I would keep it generic, but if "?" is too cryptic, we could put the original value in there like "org.apache.solr.schema.BoolField$1$1", but that might not be much better.
   


-- 
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] epugh commented on pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #1304:
URL: https://github.com/apache/solr/pull/1304#issuecomment-1400580787

   <img width="356" alt="image" src="https://user-images.githubusercontent.com/22395/214085226-599d3a5b-1b6d-40e5-a1d4-88355f5172f7.png">
   
   Ahh...   Now I see the `@todo` reason....    I wonder if the right fix is to pick the drop down, but not trigger the Luke.schema call?   Does it make sense to submit the call when we have no Field Value (Index) or Field Value (Query) defined?    
   
   Maybe we don't need the default value if we just show the page, but don't submit it to the backend, which then avoids the error message.


-- 
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] epugh commented on pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #1304:
URL: https://github.com/apache/solr/pull/1304#issuecomment-1412256804

   @stillalex I was hoping to merge this, but going to leave it open for another day and see if anyone has a tip on fixing the crave build...    I don't believe the crave build error has anything to do with the code changes, but would be nice to get all green checks!


-- 
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] stillalex commented on pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #1304:
URL: https://github.com/apache/solr/pull/1304#issuecomment-1416106834

   thank you @epugh!


-- 
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] epugh commented on pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #1304:
URL: https://github.com/apache/solr/pull/1304#issuecomment-1400570621

   <img width="528" alt="Screenshot at Jan 23 10-34-38" src="https://user-images.githubusercontent.com/22395/214083749-158ac761-ba50-4820-b221-5b52cd4293c6.png">
   
   I see the `?` from the bool...   I was trying to figure out if there is something we could put there, like "Bool" or something?   But regardless, I like how this works...     
   


-- 
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] epugh commented on pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #1304:
URL: https://github.com/apache/solr/pull/1304#issuecomment-1400650619

   > > I see the ? from the bool... I was trying to figure out if there is something we could put there, like "Bool" or something? But regardless, I like how this works...
   > 
   > I would keep it generic, but if "?" is too cryptic, we could put the original value in there like "org.apache.solr.schema.BoolField$1$1", but that might not be much better.
   
   Let's stick with `?`, cause the mouse over gives the long version.  And I suspect anything more then two or three characters would mess up the layout! 


-- 
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] epugh commented on pull request #1304: SOLR-16618 Admin UI Analysis page should include dynamic fields

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #1304:
URL: https://github.com/apache/solr/pull/1304#issuecomment-1400648984

   > 
   
   I started thinking about another param...  but that feels a bit like the `goto` statement of web dev.   How hard would it be to change the query to submit when you click the button?   Are there other places we link to the schema analysis page and have it auto run, so that requiring a button click would make the user experience worse?


-- 
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