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 2022/10/19 20:50:06 UTC
[GitHub] [solr] joel-bernstein opened a new pull request, #1094: SOLR-15733: WIP
joel-bernstein opened a new pull request, #1094:
URL: https://github.com/apache/solr/pull/1094
https://issues.apache.org/jira/browse/SOLR-XXXXX
<!--
_(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] joel-bernstein commented on pull request #1094: SOLR-15733: WIP
Posted by GitBox <gi...@apache.org>.
joel-bernstein commented on PR #1094:
URL: https://github.com/apache/solr/pull/1094#issuecomment-1285530094
@risdenk, I'm curious why it all needs to stay client?
--
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] joel-bernstein commented on pull request #1094: SOLR-15733: WIP
Posted by GitBox <gi...@apache.org>.
joel-bernstein commented on PR #1094:
URL: https://github.com/apache/solr/pull/1094#issuecomment-1285539059
It is possible that people are using a more broad set of classes on the client side. Which would indeed break if they are removed. So I see your point about waiting for 10 for splitting.
I'm ok with a solrj-streaming module.
--
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] janhoy commented on pull request #1094: SOLR-15733: WIP
Posted by GitBox <gi...@apache.org>.
janhoy commented on PR #1094:
URL: https://github.com/apache/solr/pull/1094#issuecomment-1285467317
I have used streaming in just a limited amount client-side. And the number of classes was quite overwhelming. And a bit confusing to have all the backend classes also on the client. To be honest, I ended up using the low-level builders and constructing the expressions as JSON strings in my Java client. But it would be nice to get IDE help building more complex nested streams.
So if it is possible to figure out what Bean classes and builders are needed on the client side, it would be a better end-user experience to get only those classes in `solrj-streaming`. In that case, the server-side classes could be a module for sure. Do you imagine the module would be enabled by default in 9.x and disabled by default in 10.x?
--
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] joel-bernstein closed pull request #1094: SOLR-15733: WIP
Posted by GitBox <gi...@apache.org>.
joel-bernstein closed pull request #1094: SOLR-15733: WIP
URL: https://github.com/apache/solr/pull/1094
--
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] risdenk commented on pull request #1094: SOLR-15733: WIP
Posted by GitBox <gi...@apache.org>.
risdenk commented on PR #1094:
URL: https://github.com/apache/solr/pull/1094#issuecomment-1284575694
I would have waited for after https://github.com/apache/solr/pull/953 potentially but if they are straight moves - might not be an issue. git may just handle 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] joel-bernstein commented on pull request #1094: SOLR-15733: WIP
Posted by GitBox <gi...@apache.org>.
joel-bernstein commented on PR #1094:
URL: https://github.com/apache/solr/pull/1094#issuecomment-1285488578
Ok, I like the idea of keeping a very small workable subset of classes in Solrj and moving the rest along with the /stream handler etc... to a server side module. The best supported TupleStreams Java API classes are the ones that are used by the /sql handler. So I think we can just leave these few class in Solrj. That would move 98% of the classes to a server module.
--
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] joel-bernstein commented on pull request #1094: SOLR-15733: WIP
Posted by GitBox <gi...@apache.org>.
joel-bernstein commented on PR #1094:
URL: https://github.com/apache/solr/pull/1094#issuecomment-1285498590
Another way to split the classes between server and client side module is any class that has a Java API test case can stay in Solrj. 98% of the classes only have streaming expression based test cases and they will be moved to the server side module.
--
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] janhoy commented on pull request #1094: SOLR-15733: WIP
Posted by GitBox <gi...@apache.org>.
janhoy commented on PR #1094:
URL: https://github.com/apache/solr/pull/1094#issuecomment-1285547612
Yes, similar considerations were made for `solrj-zookeeper`.
So if you start with the new `solrj-zookeeper` maven-artifact, moving all IO classes as-is, that should be back-compat with 9.0.
Next, `@Deprecate` 98% of the classes in the new `solrj-zookeeper` folder with `forRemoval="10.0"`, so users can continue using them on the client in 9.x. Then I suppose you could COPY those classes to a new server-side module, give them other package names etc, and release the new server-side module in 9.x. Then the server-side `/stream` handler will use the new backend classes from the module and not the deprecated ones. Is this possible?
--
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] risdenk commented on pull request #1094: SOLR-15733: WIP
Posted by GitBox <gi...@apache.org>.
risdenk commented on PR #1094:
URL: https://github.com/apache/solr/pull/1094#issuecomment-1285532617
user is using the streaming solrj client side java classes. - Solr 9.1 user upgrades to Solr 9.x - if all those classes are moved only to a server side module that would break in the point release.
--
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] joel-bernstein commented on pull request #1094: SOLR-15733: WIP
Posted by GitBox <gi...@apache.org>.
joel-bernstein commented on PR #1094:
URL: https://github.com/apache/solr/pull/1094#issuecomment-1285503975
As part of the solrj-streaming module I will add documentation for using the Java API. There is a massive amount of documentation for streaming expressions on the server side but nothing for Java API now.
--
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] joel-bernstein commented on pull request #1094: SOLR-15733: WIP
Posted by GitBox <gi...@apache.org>.
joel-bernstein commented on PR #1094:
URL: https://github.com/apache/solr/pull/1094#issuecomment-1285500456
If people are ok with this approach I will keep this PR alive and continue working on the server side module. Once thats done, I'll migrate a small set of classes to solrj-streaming.
--
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] HoustonPutman commented on pull request #1094: SOLR-15733: WIP
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on PR #1094:
URL: https://github.com/apache/solr/pull/1094#issuecomment-1284588558
Agreed, modules is not for solrj code, its for server modules. We don't have a solrj folder that hosts sub-folders yet (but we will starting in Solr 10), so it should just be called `solrj-streaming`. (though I really don't know why streaming expressions live in client code anyways...)
--
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] HoustonPutman commented on pull request #1094: SOLR-15733: WIP
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on PR #1094:
URL: https://github.com/apache/solr/pull/1094#issuecomment-1285573279
This sounds good to me, although I'm not sure why the streaming expressions stuff would really need to go into a module instead of solr-core. Does it require any dependencies that Solr doesn't already have?
--
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] joel-bernstein commented on pull request #1094: SOLR-15733: WIP
Posted by GitBox <gi...@apache.org>.
joel-bernstein commented on PR #1094:
URL: https://github.com/apache/solr/pull/1094#issuecomment-1285592668
@HoustonPutman, I'll focus on the move to solrj-streaming since thats what is needed for 9x.
For Solr 10 we can decide between a module or solr-core. I'm fine with either approach.
--
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] joel-bernstein commented on pull request #1094: SOLR-15733: WIP
Posted by GitBox <gi...@apache.org>.
joel-bernstein commented on PR #1094:
URL: https://github.com/apache/solr/pull/1094#issuecomment-1285561544
For this PR I'll focus on the move to solrj-streaming as that cleans things up in Solrj.
I'll open another ticket to discuss the deprecation and creation of the server side module.
--
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] joel-bernstein commented on pull request #1094: SOLR-15733: WIP
Posted by GitBox <gi...@apache.org>.
joel-bernstein commented on PR #1094:
URL: https://github.com/apache/solr/pull/1094#issuecomment-1285456616
Before I close this PR and start to work on a solrj-streaming PR lets be sure we don't want this to be server module.
The original streaming classes were designed to be a Java API using the decorator pattern to layer on new behaviors. This fit pretty well with the Solrj library which is Java based. So it was placed there.
When streaming expressions came around it became easy to support complex expressions that simply are not practical with the Java API. The /stream handler became the focal point which moved things towards are server side model. At this point I consider Streaming Expressions a server side tool.
My suggestion is that we make it a server side module and we can add Solrj support for sending a Streaming Expression to the stream handler.
The only reason I see to keep it a client side tool is to support existing clients that are using the Java API. Which may be reason enough.
One compromise would be to leave a small subset of TupleStream implementations in Solrj. Then move all the rest of the classes to a server side module.
--
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] risdenk commented on pull request #1094: SOLR-15733: WIP
Posted by GitBox <gi...@apache.org>.
risdenk commented on PR #1094:
URL: https://github.com/apache/solr/pull/1094#issuecomment-1284578666
Shouldn't these all be similar to https://github.com/apache/solr/tree/main/solr/solrj-zookeeper? looks like these ended up in modules?
--
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] risdenk commented on pull request #1094: SOLR-15733: WIP
Posted by GitBox <gi...@apache.org>.
risdenk commented on PR #1094:
URL: https://github.com/apache/solr/pull/1094#issuecomment-1285524894
@joel-bernstein for 9.x - I'm pretty sure everything needs to stay client side - so moving stuff should stay in solrj just in a separate artifact solrj-streaming.
For 10.x maybe we can see what it looks like to have a server side only module. The comments I made about solrj stuff being like maven dependencies - only applied to solrj client side. It does NOT apply to server side modules - at least not the same way.
--
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