Solr
  1. Solr
  2. SOLR-6895

Consider renaming SolrServer to SolrClient

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 5.0, 6.0
    • Fix Version/s: None
    • Component/s: SolrJ
    • Labels:
      None

      Description

      This has been niggling at me for a while. Instantiating a new SolrServer object doesn't create a server, it creates a client.

      1. SOLR-6895.patch
        1012 kB
        Alan Woodward
      2. SOLR-6895.patch
        1013 kB
        Alan Woodward

        Activity

        Hide
        Ramkumar Aiyengar added a comment -

        +1, also CloudSolrServer, LBHttpSolrServer etc.

        Show
        Ramkumar Aiyengar added a comment - +1, also CloudSolrServer , LBHttpSolrServer etc.
        Hide
        Erik Hatcher added a comment -

        However EmbeddedSolrServer does instantiate a "server", but names matter and would be good to make the appropriate changes here.

        Maybe now would be a good time to create the newly named classes and deprecate the existing ones for 5x?

        Show
        Erik Hatcher added a comment - However EmbeddedSolrServer does instantiate a "server", but names matter and would be good to make the appropriate changes here. Maybe now would be a good time to create the newly named classes and deprecate the existing ones for 5x?
        Hide
        Yonik Seeley added a comment -

        I think the origins of SolrServer instead of SolrClient go back to here:
        http://markmail.org/message/yeoqv643eootcscd
        Essentially modeling the server hence naming it Server.

        It's never felt great to me though. +1 to change to Client.
        And since we're already breaking back compat, we should look into other SolrJ cleanups as well.

        There's probably a ton of code out there that uses SolrJ though - so we should strive to either keep SolrJ4 clients compatible with Solr5, or alternately provide a simple HttpSolrServer wrapper that uses the new SolrClient.

        Show
        Yonik Seeley added a comment - I think the origins of SolrServer instead of SolrClient go back to here: http://markmail.org/message/yeoqv643eootcscd Essentially modeling the server hence naming it Server. It's never felt great to me though. +1 to change to Client. And since we're already breaking back compat, we should look into other SolrJ cleanups as well. There's probably a ton of code out there that uses SolrJ though - so we should strive to either keep SolrJ4 clients compatible with Solr5, or alternately provide a simple HttpSolrServer wrapper that uses the new SolrClient.
        Hide
        Alan Woodward added a comment -

        I think if we're going over a major version change (4 -> 5), we don't need to worry about binary compatibility here? Anybody upgrading will immediately get compiler errors, and there will be an entry in CHANGES.txt that says "SolrServer is renamed to SolrClient", etc.

        EmbeddedSolrServer should probably become something like EmbeddedServerSolrClient. Doesn't exactly trip off the tongue, but it describes what it does

        There are a bunch of other SolrJ changes and cleanups I'd like to get in for 5.0. Mainly, as I'm working on SOLR-6840, I'm trying to get more of our tests to use SolrJ, which exposes shortcomings in the API pretty quickly. I'll create issues as I come across them.

        Show
        Alan Woodward added a comment - I think if we're going over a major version change (4 -> 5), we don't need to worry about binary compatibility here? Anybody upgrading will immediately get compiler errors, and there will be an entry in CHANGES.txt that says "SolrServer is renamed to SolrClient", etc. EmbeddedSolrServer should probably become something like EmbeddedServerSolrClient. Doesn't exactly trip off the tongue, but it describes what it does There are a bunch of other SolrJ changes and cleanups I'd like to get in for 5.0. Mainly, as I'm working on SOLR-6840 , I'm trying to get more of our tests to use SolrJ, which exposes shortcomings in the API pretty quickly. I'll create issues as I come across them.
        Hide
        Yonik Seeley added a comment -

        I think if we're going over a major version change (4 -> 5), we don't need to worry about binary compatibility here?

        I'm primarily talking about compatibility with clients which have SolrJ embedded.
        But we should also consider how to make migration easy for those clients to upgrade their code as well... SolrJ (being the primary Java client) is sort of different than other parts of Solr (where reliance on Java interfaces is always expert level).

        Show
        Yonik Seeley added a comment - I think if we're going over a major version change (4 -> 5), we don't need to worry about binary compatibility here? I'm primarily talking about compatibility with clients which have SolrJ embedded. But we should also consider how to make migration easy for those clients to upgrade their code as well... SolrJ (being the primary Java client) is sort of different than other parts of Solr (where reliance on Java interfaces is always expert level).
        Hide
        Alan Woodward added a comment -

        I'm not sure I follow. If a client has SolrJ embedded, then it's got a specifically versioned JAR file in it somewhere that it's compiled against. At the moment, there aren't any changes to the SolrJ wire protocol, so clients embedding a 4.x jar will still work. If a client wants to upgrade the jar file, then they have to edit their source and recompile, but I think that's expected when going over a major version bump?

        Show
        Alan Woodward added a comment - I'm not sure I follow. If a client has SolrJ embedded, then it's got a specifically versioned JAR file in it somewhere that it's compiled against. At the moment, there aren't any changes to the SolrJ wire protocol, so clients embedding a 4.x jar will still work. If a client wants to upgrade the jar file, then they have to edit their source and recompile, but I think that's expected when going over a major version bump?
        Hide
        Yonik Seeley added a comment -

        If a client has SolrJ embedded, then it's got a specifically versioned JAR file in it somewhere that it's compiled against. At the moment, there aren't any changes to the SolrJ wire protocol, so clients embedding a 4.x jar will still work.

        Correct, this is what I was talking about when I wrote:

        There's probably a ton of code out there that uses SolrJ though - so we should strive to either keep SolrJ4 clients compatible with Solr5

        If a client wants to upgrade the jar file, then they have to edit their source and recompile, but I think that's expected when going over a major version bump?

        Yes, but we can make that easier or harder. Being a client interface, we should normally still strive for compatibility (in general), and think about how to make everyone's life easier when we do make changes. There's no hard-n-fast rules - we should do what makes the most sense.

        Show
        Yonik Seeley added a comment - If a client has SolrJ embedded, then it's got a specifically versioned JAR file in it somewhere that it's compiled against. At the moment, there aren't any changes to the SolrJ wire protocol, so clients embedding a 4.x jar will still work. Correct, this is what I was talking about when I wrote: There's probably a ton of code out there that uses SolrJ though - so we should strive to either keep SolrJ4 clients compatible with Solr5 If a client wants to upgrade the jar file, then they have to edit their source and recompile, but I think that's expected when going over a major version bump? Yes, but we can make that easier or harder. Being a client interface, we should normally still strive for compatibility (in general), and think about how to make everyone's life easier when we do make changes. There's no hard-n-fast rules - we should do what makes the most sense.
        Hide
        Alan Woodward added a comment -

        OK, I understand you now

        The name changes won't affect the wire protocol, so 4.x clients (so far!) will be able to communicate with 5.0 servers. I'll put together a patch.

        Show
        Alan Woodward added a comment - OK, I understand you now The name changes won't affect the wire protocol, so 4.x clients (so far!) will be able to communicate with 5.0 servers. I'll put together a patch.
        Hide
        Erik Hatcher added a comment -

        IMO: if we're making this change, existing classes/interfaces need to be deprecated, not just removed/renamed. Dropping the deprecated stuff can happen in 6.0, methinks. There's a big unless... unless there's going to be a 4.10.4 release that deprecates such that we can remove/rename in 5.0. Consider someone depending on Solr in an automated build by version, but they aren't going to want to change their indexing code just upgrade to Solr (Server) 5.0, which would bring in SolrJ 5.0 as well into their builds/compilation perhaps.

        Show
        Erik Hatcher added a comment - IMO: if we're making this change, existing classes/interfaces need to be deprecated, not just removed/renamed. Dropping the deprecated stuff can happen in 6.0, methinks. There's a big unless ... unless there's going to be a 4.10.4 release that deprecates such that we can remove/rename in 5.0. Consider someone depending on Solr in an automated build by version, but they aren't going to want to change their indexing code just upgrade to Solr (Server) 5.0, which would bring in SolrJ 5.0 as well into their builds/compilation perhaps.
        Hide
        Alan Woodward added a comment -

        The problem there is that this a rename, not a replacement. I'm not sure how deprecation would work - would we have to duplicate all the classes, and every method everywhere that takes a SolrServer?

        And if you're depending on Solr core classes for your build, then you're going to have to make code changes going over a major release. That's what major release versions are for!

        Show
        Alan Woodward added a comment - The problem there is that this a rename, not a replacement. I'm not sure how deprecation would work - would we have to duplicate all the classes, and every method everywhere that takes a SolrServer? And if you're depending on Solr core classes for your build, then you're going to have to make code changes going over a major release. That's what major release versions are for!
        Hide
        Alan Woodward added a comment -

        Here's the patch. I kept EmbeddedSolrServer named as it is in the end, as I think it works fine. StreamingSolrServers is renamed to StreamingSolrClients.

        Show
        Alan Woodward added a comment - Here's the patch. I kept EmbeddedSolrServer named as it is in the end, as I think it works fine. StreamingSolrServers is renamed to StreamingSolrClients.
        Hide
        Erik Hatcher added a comment -

        The problem there is that this a rename, not a replacement. I'm not sure how deprecation would work

        Create new *Client classes, copied from *Server implementation; gut and deprecate *Server implementation and have it "extend *Client" ones. Would that work for API compatibility of a Solr 4.x indexer using SolrJ 5x?

        Show
        Erik Hatcher added a comment - The problem there is that this a rename, not a replacement. I'm not sure how deprecation would work Create new *Client classes, copied from *Server implementation; gut and deprecate *Server implementation and have it "extend *Client" ones. Would that work for API compatibility of a Solr 4.x indexer using SolrJ 5x?
        Hide
        Alan Woodward added a comment -

        Oh that would work, yes. I'll amend the patch. Thanks!

        Show
        Alan Woodward added a comment - Oh that would work, yes. I'll amend the patch. Thanks!
        Hide
        Alan Woodward added a comment -

        Updated patch, with deprecations after Erik's suggestion.

        I'd like to commit this today, as it's pretty heavy and will likely get out of sync with trunk pretty quickly.

        Show
        Alan Woodward added a comment - Updated patch, with deprecations after Erik's suggestion. I'd like to commit this today, as it's pretty heavy and will likely get out of sync with trunk pretty quickly.
        Hide
        Erik Hatcher added a comment -

        Alan Woodward did you test that it works? I wonder if somehow a test could be made that ensures that SolrServer stuff still works in a 4x indexer with the renamed stuff. I have no objections, just want to be sure it's working in a backwards compatible way for 4x indexers.

        Show
        Erik Hatcher added a comment - Alan Woodward did you test that it works? I wonder if somehow a test could be made that ensures that SolrServer stuff still works in a 4x indexer with the renamed stuff. I have no objections, just want to be sure it's working in a backwards compatible way for 4x indexers.
        Hide
        Mark Miller added a comment -

        Consider renaming SolrServer to SolrClient

        +1

        Show
        Mark Miller added a comment - Consider renaming SolrServer to SolrClient +1
        Hide
        Alan Woodward added a comment -

        It'll work if you recompile against the new jar. Not sure if you just drop in the new jar to an existing indexer's classpath, I think that probably depends on the JVM and Classloader, but I don't think that's something we can guarantee anyway.

        Show
        Alan Woodward added a comment - It'll work if you recompile against the new jar. Not sure if you just drop in the new jar to an existing indexer's classpath, I think that probably depends on the JVM and Classloader, but I don't think that's something we can guarantee anyway.
        Hide
        ASF subversion and git services added a comment -

        Commit 1648697 from Alan Woodward in branch 'dev/trunk'
        [ https://svn.apache.org/r1648697 ]

        SOLR-6895: Deprecate SolrServer classes and replace with SolrClient

        Show
        ASF subversion and git services added a comment - Commit 1648697 from Alan Woodward in branch 'dev/trunk' [ https://svn.apache.org/r1648697 ] SOLR-6895 : Deprecate SolrServer classes and replace with SolrClient
        Hide
        ASF subversion and git services added a comment -

        Commit 1648706 from Alan Woodward in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1648706 ]

        SOLR-6895: Deprecate SolrServer classes and replace with SolrClient

        Show
        ASF subversion and git services added a comment - Commit 1648706 from Alan Woodward in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1648706 ] SOLR-6895 : Deprecate SolrServer classes and replace with SolrClient
        Hide
        ASF subversion and git services added a comment -

        Commit 1648710 from Alan Woodward in branch 'dev/trunk'
        [ https://svn.apache.org/r1648710 ]

        SOLR-6895: Remove SolrServer classes from trunk

        Show
        ASF subversion and git services added a comment - Commit 1648710 from Alan Woodward in branch 'dev/trunk' [ https://svn.apache.org/r1648710 ] SOLR-6895 : Remove SolrServer classes from trunk
        Hide
        Alan Woodward added a comment -

        Thanks all!

        Show
        Alan Woodward added a comment - Thanks all!
        Hide
        ASF subversion and git services added a comment -

        Commit 1648750 from Alan Woodward in branch 'dev/trunk'
        [ https://svn.apache.org/r1648750 ]

        SOLR-6895: Fix rename error in AliasIntegrationTest

        Show
        ASF subversion and git services added a comment - Commit 1648750 from Alan Woodward in branch 'dev/trunk' [ https://svn.apache.org/r1648750 ] SOLR-6895 : Fix rename error in AliasIntegrationTest
        Hide
        ASF subversion and git services added a comment -

        Commit 1648751 from Alan Woodward in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1648751 ]

        SOLR-6895: Fix rename error in AliasIntegrationTest

        Show
        ASF subversion and git services added a comment - Commit 1648751 from Alan Woodward in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1648751 ] SOLR-6895 : Fix rename error in AliasIntegrationTest
        Hide
        ASF subversion and git services added a comment -

        Commit 1648773 from Uwe Schindler in branch 'dev/trunk'
        [ https://svn.apache.org/r1648773 ]

        SOLR-6895: Fix forbidden-apis also in Maven

        Show
        ASF subversion and git services added a comment - Commit 1648773 from Uwe Schindler in branch 'dev/trunk' [ https://svn.apache.org/r1648773 ] SOLR-6895 : Fix forbidden-apis also in Maven
        Hide
        ASF subversion and git services added a comment -

        Commit 1648774 from Uwe Schindler in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1648774 ]

        Merged revision(s) 1648773 from lucene/dev/trunk:
        SOLR-6895: Fix forbidden-apis also in Maven

        Show
        ASF subversion and git services added a comment - Commit 1648774 from Uwe Schindler in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1648774 ] Merged revision(s) 1648773 from lucene/dev/trunk: SOLR-6895 : Fix forbidden-apis also in Maven

          People

          • Assignee:
            Alan Woodward
            Reporter:
            Alan Woodward
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development