Details

    • Type: Sub-task Sub-task
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.6.1, 1.7.0
    • Component/s: master, tserver
    • Labels:
      None

      Description

      The deprecation of ServerConfiguration causes two problems:

      1) eclipse is showing me ~300 warnings, so I'm missing other warnings in the noise

      2) ServerConfiguration is passed to some plug-ins (Balancer, at least), so any existing plug-ins are broken by the change.

      In the past, I would have been fine with just switching to the new class ServerConfigurationFactory but a growing user base and petabytes of existing systems gives me pause.

      Instead, I'm thinking we hoist the most popular methods to an abstract ServerConfiguration, and provide ServerConfiguationFactory as the implementation. This way the extensions continue to work.

      I'm still not sure what to do about static calls to get the SiteConfiguration.

        Issue Links

          Activity

          Hide
          Bill Havanki added a comment -

          +1 on undeprecating ServerConfiguration. Sorry about the warnings.

          IMO a "ServerConfiguration" object should be some sort of configuration object, so I'm not in favor of having the factory implement it.

          I'm happy to work with you on figuring out some solution for the plugins. ServerConfiguration isn't part of the public API, though, is it? To what extent can we practically adjust it?

          I'm working on the static SiteConfiguration calls under ACCUMULO-2603. A complicating factor is that SiteConfiguration sort of belongs in o.a.a.server.conf and not the core, as Christopher Tubbs explained in ACCUMULO-2992, but I'm not planning to address that in the near term.

          Show
          Bill Havanki added a comment - +1 on undeprecating ServerConfiguration . Sorry about the warnings. IMO a "ServerConfiguration" object should be some sort of configuration object, so I'm not in favor of having the factory implement it. I'm happy to work with you on figuring out some solution for the plugins. ServerConfiguration isn't part of the public API, though, is it? To what extent can we practically adjust it? I'm working on the static SiteConfiguration calls under ACCUMULO-2603 . A complicating factor is that SiteConfiguration sort of belongs in o.a.a.server.conf and not the core, as Christopher Tubbs explained in ACCUMULO-2992 , but I'm not planning to address that in the near term.
          Hide
          Eric Newton added a comment -

          ServerConfiguration isn't officially in the public API, but since it is passed to a couple of plug-ins, it basically is. Sure, we can say "oh, that's not in the public API", but the developers with 1000's of lines of code are not going to be happy when their custom load balancer fails to run. Also, we're becoming part of a much larger ecosystem, and forcing change on users is getting less desirable.

          I agree that the name ServerConfiguration is unfortunate, and I don't mind deprecating it. I just want to preserve backwards compatibility for extensions for as long as possible.

          We can overload the methods that take ServerConfiguration and deprecate those.

          Show
          Eric Newton added a comment - ServerConfiguration isn't officially in the public API, but since it is passed to a couple of plug-ins, it basically is. Sure, we can say "oh, that's not in the public API", but the developers with 1000's of lines of code are not going to be happy when their custom load balancer fails to run. Also, we're becoming part of a much larger ecosystem, and forcing change on users is getting less desirable. I agree that the name ServerConfiguration is unfortunate, and I don't mind deprecating it. I just want to preserve backwards compatibility for extensions for as long as possible. We can overload the methods that take ServerConfiguration and deprecate those.
          Hide
          Christopher Tubbs added a comment -

          ServerConfiguration isn't the only non-public API item that is passed to pluggable components. We really need to make interfaces for our pluggable components, to make it easier to identify and implement (and to prevent leakage of non-public API into them). I'm hoping this can be done as part of ACCUMULO-2589 (or as follow-on actions, as we identify the pluggable components).

          We should avoid putting context-specific items into the API of pluggable components. For instance, instead of giving a balancer a ServerConfiguration parameter, we can give it a Configuration object called serverConf. That would separate the plugin implementation from the internal objects Accumulo uses, and the appropriate framework can decide how much or how little configuration to actually pass in the generic Configuration object (for instance, plugins shouldn't be getting shared secrets from the server configuration).

          Show
          Christopher Tubbs added a comment - ServerConfiguration isn't the only non-public API item that is passed to pluggable components. We really need to make interfaces for our pluggable components, to make it easier to identify and implement (and to prevent leakage of non-public API into them). I'm hoping this can be done as part of ACCUMULO-2589 (or as follow-on actions, as we identify the pluggable components). We should avoid putting context-specific items into the API of pluggable components. For instance, instead of giving a balancer a ServerConfiguration parameter, we can give it a Configuration object called serverConf. That would separate the plugin implementation from the internal objects Accumulo uses, and the appropriate framework can decide how much or how little configuration to actually pass in the generic Configuration object (for instance, plugins shouldn't be getting shared secrets from the server configuration).
          Hide
          Christopher Tubbs added a comment -

          This issue still applies to the 1.6.1-SNAPSHOT branch.

          Show
          Christopher Tubbs added a comment - This issue still applies to the 1.6.1-SNAPSHOT branch.
          Hide
          Christopher Tubbs added a comment -

          Discussed with Eric Newton and it might make the most sense to simply remove the deprecation on the 1.6 branch, to get rid of the noise. The migration to the ServerConfigurationFactory is already in motion in the master branch, so I think that is the best route.

          Show
          Christopher Tubbs added a comment - Discussed with Eric Newton and it might make the most sense to simply remove the deprecation on the 1.6 branch, to get rid of the noise. The migration to the ServerConfigurationFactory is already in motion in the master branch, so I think that is the best route.

            People

            • Assignee:
              Eric Newton
              Reporter:
              Eric Newton
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0h
                0h
                Logged:
                Time Spent - 50m
                50m

                  Development