Uploaded image for project: 'Ignite'
  1. Ignite
  2. IGNITE-15747

Configuration: in-closure configuration validation

Attach filesAttach ScreenshotVotersWatch issueWatchersCreate sub-taskLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Improvement
    • Status: Resolved
    • Major
    • Resolution: Won't Fix
    • None
    • None
    • None

    Description

      Problem

      Configuration validation occurs after the code executed in the change closure, which means that inside closure it's possible to have invalid data in configuration view.
      Let's take a look at the following example. Assuming that there's following schema:

      @Config
      public class TableConfigurationSchema {
          /** Count of table partition replicas. */
          @Min(1)
          @Value(hasDefault = true)
          public int replicas = 1;
      }
      

      and following change closure

      tblCfg.change(ch -> {
          ch.changeReplicas(outherValueThatMightBeZero);
          
          ch.changePartitions(1000 / ch.replicas());
      });
      

      we might and up with an ArithmeticException: divide by zero if outherValueThatMightBeZero == 0 instead of ValidationException.

      In order not to duplicate validation logic that was specified in validators, throw ValidationException instead of ones specified in closure, etc it'll useful to guarantee that the data inside Views is valid at list at some points.

      Possible solution

      From the user experience it'll be great to validate data on every setter, however that seems to be impossible because of lack of terminal build() operation.

      Another option is to have validate() method on view and maybe value.

      On more solution, to have validate() method within some utility class instead of view.

      So, all-in-all, something similar to

      tblCfg.change(ch -> {
          ch.changeReplicas(outherValueThatMightBeZero);
          
          ch.validate();
          // or
          SomeCfgUtils.validate(ch);
      
          ch.changePartitions(1000 / ch.replicas());
      });
      

      is expected.

      Attachments

        Issue Links

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            Unassigned Unassigned
            alapin Alexander Lapin
            Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment