Uploaded image for project: 'Tajo'
  1. Tajo
  2. TAJO-1114

Improve ConfVars (SessionVar) to take a validator interface to check its input.

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.10.0
    • Component/s: None
    • Labels:
      None

      Description

      Each task uses session variables from client or configs from tajo-site.xml. The session variables and configs are used in task during query processing.

      If invalid configs or invalid session variables are used for a query, the error will be caused in a runtime instead of query init phase. It's bad situation. Invalid values must be detected at the initialization phase.

      Because each session variable or config varies in its value type and value range, It would be great if each ConfigKey takes a validator for its value and Tajo uses validators for it.

        Activity

        Hide
        ykrips Jihun Kang added a comment -

        It looks very interesting feature. I will start to work on this issue.

        Show
        ykrips Jihun Kang added a comment - It looks very interesting feature. I will start to work on this issue.
        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user ykrips opened a pull request:

        https://github.com/apache/tajo/pull/206

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        You can merge this pull request into a Git repository by running:

        $ git pull https://github.com/ykrips/tajo TAJO-1114

        Alternatively you can review and apply these changes as the patch at:

        https://github.com/apache/tajo/pull/206.patch

        To close this pull request, make a commit to your master/trunk branch
        with (at least) the following in the commit message:

        This closes #206


        commit 412e2e6b8fcd208ded929df100d2ee80ee7a0281
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-17T09:53:13Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit 44b991b43318da6d91758c3ef43232bd71414352
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T06:47:34Z

        Merge remote-tracking branch 'upstream/master' into TAJO-1114

        commit 76720f853dd22b245207d2de8e49fcc9554562f5
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T07:13:01Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit f99e7edcfa51c2955da7a54af4a2edf7b11c6068
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T07:13:53Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit 0ad448d1e51a35778b527a952dbd2c36345579e5
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T07:15:00Z

        Merge remote-tracking branch 'upstream/master' into TAJO-1114

        commit ce0fb6d44e5782c28618b735d95b10c57d3f4692
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T12:47:27Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit 3f3a23a138cc523316cd2b28f1251921a55d404e
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T13:02:30Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user ykrips opened a pull request: https://github.com/apache/tajo/pull/206 TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. You can merge this pull request into a Git repository by running: $ git pull https://github.com/ykrips/tajo TAJO-1114 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/206.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #206 commit 412e2e6b8fcd208ded929df100d2ee80ee7a0281 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-17T09:53:13Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit 44b991b43318da6d91758c3ef43232bd71414352 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T06:47:34Z Merge remote-tracking branch 'upstream/master' into TAJO-1114 commit 76720f853dd22b245207d2de8e49fcc9554562f5 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T07:13:01Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit f99e7edcfa51c2955da7a54af4a2edf7b11c6068 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T07:13:53Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit 0ad448d1e51a35778b527a952dbd2c36345579e5 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T07:15:00Z Merge remote-tracking branch 'upstream/master' into TAJO-1114 commit ce0fb6d44e5782c28618b735d95b10c57d3f4692 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T12:47:27Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit 3f3a23a138cc523316cd2b28f1251921a55d404e Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T13:02:30Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/206#issuecomment-60461282

        Even through it needs rebase, it was easy to merge the patch against the latest revision.

        I'm reviewing this patch. I'll leave comments soon.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/206#issuecomment-60461282 Even through it needs rebase, it was easy to merge the patch against the latest revision. I'm reviewing this patch. I'll leave comments soon.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on a diff in the pull request:

        https://github.com/apache/tajo/pull/206#discussion_r19374125

        — Diff: tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java —
        @@ -143,54 +156,61 @@ public static int setDateOrder(int dateOrder) {
        ///////////////////////////////////////////////////////////////////////////////////////

        // a username for a running Tajo cluster

        • ROOT_DIR("tajo.rootdir", "file:///tmp/tajo-$ {user.name}/"),
          - USERNAME("tajo.username", "${user.name}

          "),
          + ROOT_DIR("tajo.rootdir", "file:///tmp/tajo-$

          {user.name}/",
          + Validators.groups(Validators.notNull(), Validators.pathUrl())),
          + USERNAME("tajo.username", "${user.name}

          ",
          + Validators.groups(Validators.notNull(), Validators.shellVar())),

            • End diff –

        Shell variable (i.e., ```$

        {user.name}

        ) is translated in Hadoop's Configuration. Actually, this config is just a string.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on a diff in the pull request: https://github.com/apache/tajo/pull/206#discussion_r19374125 — Diff: tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java — @@ -143,54 +156,61 @@ public static int setDateOrder(int dateOrder) { /////////////////////////////////////////////////////////////////////////////////////// // a username for a running Tajo cluster ROOT_DIR("tajo.rootdir", "file:///tmp/tajo-$ {user.name}/"), - USERNAME("tajo.username", "${user.name} "), + ROOT_DIR("tajo.rootdir", "file:///tmp/tajo-$ {user.name}/", + Validators.groups(Validators.notNull(), Validators.pathUrl())), + USERNAME("tajo.username", "${user.name} ", + Validators.groups(Validators.notNull(), Validators.shellVar())), End diff – Shell variable (i.e., ```$ {user.name} ) is translated in Hadoop's Configuration. Actually, this config is just a string.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/206#issuecomment-60473494

        I greatly appreciate your contribution. The patch looks really great. I love your design.

        Actually, I didn't mention how we validate each config key. Of course, it is out of scope of this issue. Now, I just share it with you.

        In my plan, Tajo will validate configs in two ways.

        The first way is that TajoMaster validates each config when it starts up. If TajoMaster finds invalid config which probably will cause query runtime errors, TajoMaster will shutdown immediately and prints out detailed error messages. If so, we can earlier prohibit some kind of query runtime errors.

        The second one is that TajoMaster checks each session setting action by client. If a client sets an invalid session variable, it will show some errors and prohibit putting the invalid variable into session variables.

        If you fix the validator for ```tajo.username``` and rebase it, I'll finish the review.

        Best regards,
        Hyunsik

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/206#issuecomment-60473494 I greatly appreciate your contribution. The patch looks really great. I love your design. Actually, I didn't mention how we validate each config key. Of course, it is out of scope of this issue. Now, I just share it with you. In my plan, Tajo will validate configs in two ways. The first way is that TajoMaster validates each config when it starts up. If TajoMaster finds invalid config which probably will cause query runtime errors, TajoMaster will shutdown immediately and prints out detailed error messages. If so, we can earlier prohibit some kind of query runtime errors. The second one is that TajoMaster checks each session setting action by client. If a client sets an invalid session variable, it will show some errors and prohibit putting the invalid variable into session variables. If you fix the validator for ```tajo.username``` and rebase it, I'll finish the review. Best regards, Hyunsik
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user ykrips commented on the pull request:

        https://github.com/apache/tajo/pull/206#issuecomment-60557163

        Hello @hyunsik ,
        I will fix some missing points for your comments, and will rebase it.
        On my implementation, I have overrided some functions of Configuration class to raise exceptions when invalid values are inserted. However, with your explanation, I feel that TajoMaster will throw exceptions, not TajoConf. I just wonder if I correctly understand your strategy.

        Show
        githubbot ASF GitHub Bot added a comment - Github user ykrips commented on the pull request: https://github.com/apache/tajo/pull/206#issuecomment-60557163 Hello @hyunsik , I will fix some missing points for your comments, and will rebase it. On my implementation, I have overrided some functions of Configuration class to raise exceptions when invalid values are inserted. However, with your explanation, I feel that TajoMaster will throw exceptions, not TajoConf. I just wonder if I correctly understand your strategy.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user ykrips commented on the pull request:

        https://github.com/apache/tajo/pull/206#issuecomment-60565026

        Hello @hyunsik ,
        I am apologized for my mistake on this pull request. I trying to rebase my local branch to github repository. But it corrupt ref files or any other files on my git repository. I'm sorry again for my mistake.

        Show
        githubbot ASF GitHub Bot added a comment - Github user ykrips commented on the pull request: https://github.com/apache/tajo/pull/206#issuecomment-60565026 Hello @hyunsik , I am apologized for my mistake on this pull request. I trying to rebase my local branch to github repository. But it corrupt ref files or any other files on my git repository. I'm sorry again for my mistake.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user ykrips closed the pull request at:

        https://github.com/apache/tajo/pull/206

        Show
        githubbot ASF GitHub Bot added a comment - Github user ykrips closed the pull request at: https://github.com/apache/tajo/pull/206
        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user ykrips opened a pull request:

        https://github.com/apache/tajo/pull/214

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        This pull request continues the previous pull request.
        Previous pull request can be found in this link.
        https://github.com/apache/tajo/pull/206

        You can merge this pull request into a Git repository by running:

        $ git pull https://github.com/ykrips/tajo TAJO-1114

        Alternatively you can review and apply these changes as the patch at:

        https://github.com/apache/tajo/pull/214.patch

        To close this pull request, make a commit to your master/trunk branch
        with (at least) the following in the commit message:

        This closes #214


        commit 412e2e6b8fcd208ded929df100d2ee80ee7a0281
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-17T09:53:13Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit 44b991b43318da6d91758c3ef43232bd71414352
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T06:47:34Z

        Merge remote-tracking branch 'upstream/master' into TAJO-1114

        commit 76720f853dd22b245207d2de8e49fcc9554562f5
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T07:13:01Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit f99e7edcfa51c2955da7a54af4a2edf7b11c6068
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T07:13:53Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit 0ad448d1e51a35778b527a952dbd2c36345579e5
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T07:15:00Z

        Merge remote-tracking branch 'upstream/master' into TAJO-1114

        commit ce0fb6d44e5782c28618b735d95b10c57d3f4692
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T12:47:27Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit 3f3a23a138cc523316cd2b28f1251921a55d404e
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T13:02:30Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit b953102a48bb75136172a584d95191273a386903
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-17T09:53:13Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit c510067ce5665a431230e5e56efa574f014bb85d
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T07:13:01Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit 9d666ef75662c36e825b6bcdc1cd22e30d6c59af
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T07:13:53Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit 1b5088bcf515f2e6706be1d1dbec515a9885b8e4
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T12:47:27Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit 7c92470a100732434e3aa453978c5b498eecca6e
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T13:02:30Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit b49df082a32d2bb5a987c7444b0b4b0e94fa8836
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-27T08:52:58Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit cc5d128a3a5c2701ed119b7d6de9b8aa81799c08
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-27T09:00:32Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit 5b5fc3995cba4fced25563b35e1966db5a8812d9
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-27T09:10:41Z

        Removed validate functions from OverridableConf

        commit d0ee8cd3c52cec0588af53ad8554e40ba437f711
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-27T09:14:09Z

        Removed unused conf


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user ykrips opened a pull request: https://github.com/apache/tajo/pull/214 TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. This pull request continues the previous pull request. Previous pull request can be found in this link. https://github.com/apache/tajo/pull/206 You can merge this pull request into a Git repository by running: $ git pull https://github.com/ykrips/tajo TAJO-1114 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/214.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #214 commit 412e2e6b8fcd208ded929df100d2ee80ee7a0281 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-17T09:53:13Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit 44b991b43318da6d91758c3ef43232bd71414352 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T06:47:34Z Merge remote-tracking branch 'upstream/master' into TAJO-1114 commit 76720f853dd22b245207d2de8e49fcc9554562f5 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T07:13:01Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit f99e7edcfa51c2955da7a54af4a2edf7b11c6068 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T07:13:53Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit 0ad448d1e51a35778b527a952dbd2c36345579e5 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T07:15:00Z Merge remote-tracking branch 'upstream/master' into TAJO-1114 commit ce0fb6d44e5782c28618b735d95b10c57d3f4692 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T12:47:27Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit 3f3a23a138cc523316cd2b28f1251921a55d404e Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T13:02:30Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit b953102a48bb75136172a584d95191273a386903 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-17T09:53:13Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit c510067ce5665a431230e5e56efa574f014bb85d Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T07:13:01Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit 9d666ef75662c36e825b6bcdc1cd22e30d6c59af Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T07:13:53Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit 1b5088bcf515f2e6706be1d1dbec515a9885b8e4 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T12:47:27Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit 7c92470a100732434e3aa453978c5b498eecca6e Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T13:02:30Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit b49df082a32d2bb5a987c7444b0b4b0e94fa8836 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-27T08:52:58Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit cc5d128a3a5c2701ed119b7d6de9b8aa81799c08 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-27T09:00:32Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit 5b5fc3995cba4fced25563b35e1966db5a8812d9 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-27T09:10:41Z Removed validate functions from OverridableConf commit d0ee8cd3c52cec0588af53ad8554e40ba437f711 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-27T09:14:09Z Removed unused conf
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/206#issuecomment-60566349

        @ykrips

        No problem
        I'll leave the comment about your question in #214.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/206#issuecomment-60566349 @ykrips No problem I'll leave the comment about your question in #214.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/214#issuecomment-60572743

        > Hello @hyunsik ,
        I will fix some missing points for your comments, and will rebase it.
        On my implementation, I have overrided some functions of Configuration class to raise exceptions when invalid values are inserted. However, with your explanation, I feel that TajoMaster will throw exceptions, not TajoConf. I just wonder if I correctly understand your strategy.

        Hi @ykrips,

        Thank you for your work again! I leave comments here for your question in #206.

        The main reason why TajoMaster needs to check all configs is that we need to limit the scope of runtime exception.

        The background is as follows:

        • TajoConf is a subclass of Hadoop Configuration.
        • Configuration loads ```-site.xml``` and ```-default.xml``` in Java classpaths when it is initialized.
        • Coniguration internally will use set(String, String) when it initialized.
        • According to our policy of TajoConf usage, we *do not modify any config in runtime* except for unit tests.
        • In other words, they are constant configs loaded when Tajo cluster starts up.
        • As a result, validateProperty() will be mostly used when TajoConf is initialized.

        Consequently, we need to handle exception thrown by the constructor of TajoConf. TajoConf is initialized in lots of parts, and exception handling against constructor may be not good in my opinion.

        In my view. it would be enough to validate configs in three entry points of TajoMaster, TajoWorker and TajoClient. They are main components in a Tajo cluster. (In previous, I mentioned only TajoMaster. We actually need config validation in two more components.)

        Later, we can improve three components (TajoMaster, TajoWorker, and TajoClient) to have some diagnose phase to check all configs, connectivities among Tajo components and states of components. Your proposed patch would be a part of the diagnose phase.

        Thanks,
        Hyunsik

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/214#issuecomment-60572743 > Hello @hyunsik , I will fix some missing points for your comments, and will rebase it. On my implementation, I have overrided some functions of Configuration class to raise exceptions when invalid values are inserted. However, with your explanation, I feel that TajoMaster will throw exceptions, not TajoConf. I just wonder if I correctly understand your strategy. Hi @ykrips, Thank you for your work again! I leave comments here for your question in #206. The main reason why TajoMaster needs to check all configs is that we need to limit the scope of runtime exception. The background is as follows: TajoConf is a subclass of Hadoop Configuration. Configuration loads ``` -site.xml``` and ``` -default.xml``` in Java classpaths when it is initialized. Coniguration internally will use set(String, String) when it initialized. According to our policy of TajoConf usage, we * do not modify any config in runtime * except for unit tests. In other words, they are constant configs loaded when Tajo cluster starts up. As a result, validateProperty() will be mostly used when TajoConf is initialized. Consequently, we need to handle exception thrown by the constructor of TajoConf. TajoConf is initialized in lots of parts, and exception handling against constructor may be not good in my opinion. In my view. it would be enough to validate configs in three entry points of TajoMaster, TajoWorker and TajoClient. They are main components in a Tajo cluster. (In previous, I mentioned only TajoMaster. We actually need config validation in two more components.) Later, we can improve three components (TajoMaster, TajoWorker, and TajoClient) to have some diagnose phase to check all configs, connectivities among Tajo components and states of components. Your proposed patch would be a part of the diagnose phase. Thanks, Hyunsik
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user ykrips commented on the pull request:

        https://github.com/apache/tajo/pull/214#issuecomment-60580810

        @hyunsik , Thank you for clarifying my understandings on this validator design. I could have clear understanding on, and agree with your design goal and strategy. I'll finish up commits with my understandings and it will end up in a short time.

        Show
        githubbot ASF GitHub Bot added a comment - Github user ykrips commented on the pull request: https://github.com/apache/tajo/pull/214#issuecomment-60580810 @hyunsik , Thank you for clarifying my understandings on this validator design. I could have clear understanding on, and agree with your design goal and strategy. I'll finish up commits with my understandings and it will end up in a short time.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/214#issuecomment-60628586

        +1

        The patch looks great to me. I'll commit it shortly.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/214#issuecomment-60628586 +1 The patch looks great to me. I'll commit it shortly.
        Hide
        hyunsik Hyunsik Choi added a comment -

        I just committed the patch to master branch. I appreciate your contribution.

        Show
        hyunsik Hyunsik Choi added a comment - I just committed the patch to master branch. I appreciate your contribution.
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Tajo-master-build #422 (See https://builds.apache.org/job/Tajo-master-build/422/)
        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input. (Jihun Kang via hyunsik) (hyunsik: rev ee89c65b44142ca640020339e7ab5a608c1a8bf9)

        • CHANGES
        • tajo-common/src/main/java/org/apache/tajo/SessionVars.java
        • tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java
        • tajo-common/src/main/java/org/apache/tajo/validation/LengthValidator.java
        • tajo-common/pom.xml
        • tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolation.java
        • tajo-common/src/main/java/org/apache/tajo/validation/ShellVariableValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/ClassValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/GroupValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/PathValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/RangeValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/JavaStringValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/MinValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/BooleanValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/PatternValidator.java
        • tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java
        • tajo-common/src/main/java/org/apache/tajo/validation/NotNullValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/NetworkAddressValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/AbstractValidator.java
        • tajo-common/src/main/java/org/apache/tajo/ConfigKey.java
        • tajo-core/src/main/java/org/apache/tajo/engine/query/QueryContext.java
        • tajo-common/src/main/java/org/apache/tajo/validation/Validators.java
        • tajo-common/src/main/java/org/apache/tajo/validation/Validator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolationException.java
        • tajo-common/src/main/java/org/apache/tajo/validation/MaxValidator.java
        • tajo-common/src/main/java/org/apache/tajo/util/NumberUtil.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Tajo-master-build #422 (See https://builds.apache.org/job/Tajo-master-build/422/ ) TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. (Jihun Kang via hyunsik) (hyunsik: rev ee89c65b44142ca640020339e7ab5a608c1a8bf9) CHANGES tajo-common/src/main/java/org/apache/tajo/SessionVars.java tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java tajo-common/src/main/java/org/apache/tajo/validation/LengthValidator.java tajo-common/pom.xml tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolation.java tajo-common/src/main/java/org/apache/tajo/validation/ShellVariableValidator.java tajo-common/src/main/java/org/apache/tajo/validation/ClassValidator.java tajo-common/src/main/java/org/apache/tajo/validation/GroupValidator.java tajo-common/src/main/java/org/apache/tajo/validation/PathValidator.java tajo-common/src/main/java/org/apache/tajo/validation/RangeValidator.java tajo-common/src/main/java/org/apache/tajo/validation/JavaStringValidator.java tajo-common/src/main/java/org/apache/tajo/validation/MinValidator.java tajo-common/src/main/java/org/apache/tajo/validation/BooleanValidator.java tajo-common/src/main/java/org/apache/tajo/validation/PatternValidator.java tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java tajo-common/src/main/java/org/apache/tajo/validation/NotNullValidator.java tajo-common/src/main/java/org/apache/tajo/validation/NetworkAddressValidator.java tajo-common/src/main/java/org/apache/tajo/validation/AbstractValidator.java tajo-common/src/main/java/org/apache/tajo/ConfigKey.java tajo-core/src/main/java/org/apache/tajo/engine/query/QueryContext.java tajo-common/src/main/java/org/apache/tajo/validation/Validators.java tajo-common/src/main/java/org/apache/tajo/validation/Validator.java tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolationException.java tajo-common/src/main/java/org/apache/tajo/validation/MaxValidator.java tajo-common/src/main/java/org/apache/tajo/util/NumberUtil.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Tajo-master-CODEGEN-build #64 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/64/)
        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input. (Jihun Kang via hyunsik) (hyunsik: rev ee89c65b44142ca640020339e7ab5a608c1a8bf9)

        • tajo-common/src/main/java/org/apache/tajo/util/NumberUtil.java
        • tajo-common/src/main/java/org/apache/tajo/validation/NotNullValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/JavaStringValidator.java
        • CHANGES
        • tajo-common/src/main/java/org/apache/tajo/SessionVars.java
        • tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java
        • tajo-common/src/main/java/org/apache/tajo/validation/Validators.java
        • tajo-common/src/main/java/org/apache/tajo/validation/Validator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/PatternValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/NetworkAddressValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/PathValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/MaxValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/ShellVariableValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/RangeValidator.java
        • tajo-common/pom.xml
        • tajo-common/src/main/java/org/apache/tajo/validation/GroupValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolationException.java
        • tajo-core/src/main/java/org/apache/tajo/engine/query/QueryContext.java
        • tajo-common/src/main/java/org/apache/tajo/validation/LengthValidator.java
        • tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java
        • tajo-common/src/main/java/org/apache/tajo/ConfigKey.java
        • tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolation.java
        • tajo-common/src/main/java/org/apache/tajo/validation/MinValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/ClassValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/AbstractValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/BooleanValidator.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Tajo-master-CODEGEN-build #64 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/64/ ) TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. (Jihun Kang via hyunsik) (hyunsik: rev ee89c65b44142ca640020339e7ab5a608c1a8bf9) tajo-common/src/main/java/org/apache/tajo/util/NumberUtil.java tajo-common/src/main/java/org/apache/tajo/validation/NotNullValidator.java tajo-common/src/main/java/org/apache/tajo/validation/JavaStringValidator.java CHANGES tajo-common/src/main/java/org/apache/tajo/SessionVars.java tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java tajo-common/src/main/java/org/apache/tajo/validation/Validators.java tajo-common/src/main/java/org/apache/tajo/validation/Validator.java tajo-common/src/main/java/org/apache/tajo/validation/PatternValidator.java tajo-common/src/main/java/org/apache/tajo/validation/NetworkAddressValidator.java tajo-common/src/main/java/org/apache/tajo/validation/PathValidator.java tajo-common/src/main/java/org/apache/tajo/validation/MaxValidator.java tajo-common/src/main/java/org/apache/tajo/validation/ShellVariableValidator.java tajo-common/src/main/java/org/apache/tajo/validation/RangeValidator.java tajo-common/pom.xml tajo-common/src/main/java/org/apache/tajo/validation/GroupValidator.java tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolationException.java tajo-core/src/main/java/org/apache/tajo/engine/query/QueryContext.java tajo-common/src/main/java/org/apache/tajo/validation/LengthValidator.java tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java tajo-common/src/main/java/org/apache/tajo/ConfigKey.java tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolation.java tajo-common/src/main/java/org/apache/tajo/validation/MinValidator.java tajo-common/src/main/java/org/apache/tajo/validation/ClassValidator.java tajo-common/src/main/java/org/apache/tajo/validation/AbstractValidator.java tajo-common/src/main/java/org/apache/tajo/validation/BooleanValidator.java
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

        https://github.com/apache/tajo/pull/214

        Show
        githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/tajo/pull/214
        Hide
        hyunsik Hyunsik Choi added a comment -

        It's build failure seems to be caused by Java 1.6. I'll fix it and then commit it with the fix.

        Show
        hyunsik Hyunsik Choi added a comment - It's build failure seems to be caused by Java 1.6. I'll fix it and then commit it with the fix.
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Tajo-master-CODEGEN-build #65 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/65/)
        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input. (fixed compilation error) (hyunsik: rev 7a65763554e556498b6d52dd2685636c3b6584b3)

        • tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Tajo-master-CODEGEN-build #65 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/65/ ) TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. (fixed compilation error) (hyunsik: rev 7a65763554e556498b6d52dd2685636c3b6584b3) tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Tajo-master-build #423 (See https://builds.apache.org/job/Tajo-master-build/423/)
        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input. (fixed compilation error) (hyunsik: rev 7a65763554e556498b6d52dd2685636c3b6584b3)

        • tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Tajo-master-build #423 (See https://builds.apache.org/job/Tajo-master-build/423/ ) TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. (fixed compilation error) (hyunsik: rev 7a65763554e556498b6d52dd2685636c3b6584b3) tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java
        Show
        hyunsik Hyunsik Choi added a comment - It appears to be a JDK bug. http://bugs.java.com/bugdatabase/view_bug.do?bug_id=7035413 http://bugs.java.com/bugdatabase/view_bug.do?bug_id=7034548
        Hide
        hyunsik Hyunsik Choi added a comment -

        Hi Jihun,

        Actually, I expect that this problem is just a generic support of JVM 1.6. But, in Java 1.6, this patch does not work properly after all compilation problems are fixed.

        You can see the error logs at:

        So, I reopened this jira issue. Could you check it in Java 1.6?

        Thanks,
        Hyunsik

        Show
        hyunsik Hyunsik Choi added a comment - Hi Jihun, Actually, I expect that this problem is just a generic support of JVM 1.6. But, in Java 1.6, this patch does not work properly after all compilation problems are fixed. You can see the error logs at: https://builds.apache.org/job/Tajo-master-build/425/console So, I reopened this jira issue. Could you check it in Java 1.6? Thanks, Hyunsik
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-build #426 (See https://builds.apache.org/job/Tajo-master-build/426/)
        Revert "TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input. (fixed compilation error)" (hyunsik: rev 58b8eb45c0ef39047fb1738c10e2411a8cc0dc5e)

        • tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java
          Revert "TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input. (Jihun Kang via hyunsik)" (hyunsik: rev 8741e68291cb8dccb64d4c2e0b872cc56a4dbf3f)
        • tajo-common/src/main/java/org/apache/tajo/validation/MaxValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolation.java
        • tajo-common/src/main/java/org/apache/tajo/ConfigKey.java
        • tajo-common/src/main/java/org/apache/tajo/validation/RangeValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/LengthValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/ShellVariableValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/NotNullValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/JavaStringValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/GroupValidator.java
        • tajo-common/src/main/java/org/apache/tajo/SessionVars.java
        • tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolationException.java
        • tajo-common/src/main/java/org/apache/tajo/validation/NetworkAddressValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/PatternValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/Validators.java
        • tajo-common/src/main/java/org/apache/tajo/util/NumberUtil.java
        • tajo-common/src/main/java/org/apache/tajo/validation/MinValidator.java
        • tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java
        • CHANGES
        • tajo-common/src/main/java/org/apache/tajo/validation/PathValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/BooleanValidator.java
        • tajo-core/src/main/java/org/apache/tajo/engine/query/QueryContext.java
        • tajo-common/src/main/java/org/apache/tajo/validation/ClassValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/AbstractValidator.java
        • tajo-common/pom.xml
        • tajo-common/src/main/java/org/apache/tajo/validation/Validator.java
        • tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #426 (See https://builds.apache.org/job/Tajo-master-build/426/ ) Revert " TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. (fixed compilation error)" (hyunsik: rev 58b8eb45c0ef39047fb1738c10e2411a8cc0dc5e) tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java Revert " TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. (Jihun Kang via hyunsik)" (hyunsik: rev 8741e68291cb8dccb64d4c2e0b872cc56a4dbf3f) tajo-common/src/main/java/org/apache/tajo/validation/MaxValidator.java tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolation.java tajo-common/src/main/java/org/apache/tajo/ConfigKey.java tajo-common/src/main/java/org/apache/tajo/validation/RangeValidator.java tajo-common/src/main/java/org/apache/tajo/validation/LengthValidator.java tajo-common/src/main/java/org/apache/tajo/validation/ShellVariableValidator.java tajo-common/src/main/java/org/apache/tajo/validation/NotNullValidator.java tajo-common/src/main/java/org/apache/tajo/validation/JavaStringValidator.java tajo-common/src/main/java/org/apache/tajo/validation/GroupValidator.java tajo-common/src/main/java/org/apache/tajo/SessionVars.java tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolationException.java tajo-common/src/main/java/org/apache/tajo/validation/NetworkAddressValidator.java tajo-common/src/main/java/org/apache/tajo/validation/PatternValidator.java tajo-common/src/main/java/org/apache/tajo/validation/Validators.java tajo-common/src/main/java/org/apache/tajo/util/NumberUtil.java tajo-common/src/main/java/org/apache/tajo/validation/MinValidator.java tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java CHANGES tajo-common/src/main/java/org/apache/tajo/validation/PathValidator.java tajo-common/src/main/java/org/apache/tajo/validation/BooleanValidator.java tajo-core/src/main/java/org/apache/tajo/engine/query/QueryContext.java tajo-common/src/main/java/org/apache/tajo/validation/ClassValidator.java tajo-common/src/main/java/org/apache/tajo/validation/AbstractValidator.java tajo-common/pom.xml tajo-common/src/main/java/org/apache/tajo/validation/Validator.java tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-CODEGEN-build #68 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/68/)
        Revert "TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input. (fixed compilation error)" (hyunsik: rev 58b8eb45c0ef39047fb1738c10e2411a8cc0dc5e)

        • tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java
          Revert "TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input. (Jihun Kang via hyunsik)" (hyunsik: rev 8741e68291cb8dccb64d4c2e0b872cc56a4dbf3f)
        • tajo-common/src/main/java/org/apache/tajo/validation/LengthValidator.java
        • tajo-common/src/main/java/org/apache/tajo/SessionVars.java
        • tajo-common/src/main/java/org/apache/tajo/validation/Validators.java
        • tajo-core/src/main/java/org/apache/tajo/engine/query/QueryContext.java
        • tajo-common/src/main/java/org/apache/tajo/validation/PatternValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/ClassValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/MaxValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/AbstractValidator.java
        • tajo-common/src/main/java/org/apache/tajo/util/NumberUtil.java
        • tajo-common/src/main/java/org/apache/tajo/validation/NetworkAddressValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/PathValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolation.java
        • CHANGES
        • tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolationException.java
        • tajo-common/src/main/java/org/apache/tajo/validation/ShellVariableValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/MinValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/GroupValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/RangeValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/JavaStringValidator.java
        • tajo-common/pom.xml
        • tajo-common/src/main/java/org/apache/tajo/validation/Validator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/BooleanValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/NotNullValidator.java
        • tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java
        • tajo-common/src/main/java/org/apache/tajo/ConfigKey.java
        • tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-CODEGEN-build #68 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/68/ ) Revert " TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. (fixed compilation error)" (hyunsik: rev 58b8eb45c0ef39047fb1738c10e2411a8cc0dc5e) tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java Revert " TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. (Jihun Kang via hyunsik)" (hyunsik: rev 8741e68291cb8dccb64d4c2e0b872cc56a4dbf3f) tajo-common/src/main/java/org/apache/tajo/validation/LengthValidator.java tajo-common/src/main/java/org/apache/tajo/SessionVars.java tajo-common/src/main/java/org/apache/tajo/validation/Validators.java tajo-core/src/main/java/org/apache/tajo/engine/query/QueryContext.java tajo-common/src/main/java/org/apache/tajo/validation/PatternValidator.java tajo-common/src/main/java/org/apache/tajo/validation/ClassValidator.java tajo-common/src/main/java/org/apache/tajo/validation/MaxValidator.java tajo-common/src/main/java/org/apache/tajo/validation/AbstractValidator.java tajo-common/src/main/java/org/apache/tajo/util/NumberUtil.java tajo-common/src/main/java/org/apache/tajo/validation/NetworkAddressValidator.java tajo-common/src/main/java/org/apache/tajo/validation/PathValidator.java tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolation.java CHANGES tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolationException.java tajo-common/src/main/java/org/apache/tajo/validation/ShellVariableValidator.java tajo-common/src/main/java/org/apache/tajo/validation/MinValidator.java tajo-common/src/main/java/org/apache/tajo/validation/GroupValidator.java tajo-common/src/main/java/org/apache/tajo/validation/RangeValidator.java tajo-common/src/main/java/org/apache/tajo/validation/JavaStringValidator.java tajo-common/pom.xml tajo-common/src/main/java/org/apache/tajo/validation/Validator.java tajo-common/src/main/java/org/apache/tajo/validation/BooleanValidator.java tajo-common/src/main/java/org/apache/tajo/validation/NotNullValidator.java tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java tajo-common/src/main/java/org/apache/tajo/ConfigKey.java tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java
        Hide
        ykrips Jihun Kang added a comment - - edited

        I found some compilation errors when using hamcrest-core library. It may comes with JDK6 compiler limitation, and I added some workarounds on my test case. I have tested on JDK 1.6.0_45 and 1.7.0_67.

        Show
        ykrips Jihun Kang added a comment - - edited I found some compilation errors when using hamcrest-core library. It may comes with JDK6 compiler limitation, and I added some workarounds on my test case. I have tested on JDK 1.6.0_45 and 1.7.0_67. https://code.google.com/p/hamcrest/issues/detail?id=100#c16
        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user ykrips opened a pull request:

        https://github.com/apache/tajo/pull/218

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        I cannot expect that generic signature of IsCollectionContaining is broken. After the quick review on source code fo IsCollectionContaining, I found the broken point on this class, and add a workaround function on the test case.

        You can merge this pull request into a Git repository by running:

        $ git pull https://github.com/ykrips/tajo TAJO-1114

        Alternatively you can review and apply these changes as the patch at:

        https://github.com/apache/tajo/pull/218.patch

        To close this pull request, make a commit to your master/trunk branch
        with (at least) the following in the commit message:

        This closes #218


        commit 412e2e6b8fcd208ded929df100d2ee80ee7a0281
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-17T09:53:13Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit 44b991b43318da6d91758c3ef43232bd71414352
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T06:47:34Z

        Merge remote-tracking branch 'upstream/master' into TAJO-1114

        commit 76720f853dd22b245207d2de8e49fcc9554562f5
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T07:13:01Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit f99e7edcfa51c2955da7a54af4a2edf7b11c6068
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T07:13:53Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit 0ad448d1e51a35778b527a952dbd2c36345579e5
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T07:15:00Z

        Merge remote-tracking branch 'upstream/master' into TAJO-1114

        commit ce0fb6d44e5782c28618b735d95b10c57d3f4692
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T12:47:27Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit 3f3a23a138cc523316cd2b28f1251921a55d404e
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T13:02:30Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit b953102a48bb75136172a584d95191273a386903
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-17T09:53:13Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit c510067ce5665a431230e5e56efa574f014bb85d
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T07:13:01Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit 9d666ef75662c36e825b6bcdc1cd22e30d6c59af
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T07:13:53Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit 1b5088bcf515f2e6706be1d1dbec515a9885b8e4
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T12:47:27Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit 7c92470a100732434e3aa453978c5b498eecca6e
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-20T13:02:30Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit b49df082a32d2bb5a987c7444b0b4b0e94fa8836
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-27T08:52:58Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit cc5d128a3a5c2701ed119b7d6de9b8aa81799c08
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-27T09:00:32Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit 5b5fc3995cba4fced25563b35e1966db5a8812d9
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-27T09:10:41Z

        Removed validate functions from OverridableConf

        commit d0ee8cd3c52cec0588af53ad8554e40ba437f711
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-27T09:14:09Z

        Removed unused conf

        commit 5a9b44b1f6dc6d960721b9a5154cbd33df389882
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-27T09:31:27Z

        Merge branch 'TAJO-1114' of https://github.com/ykrips/tajo into TAJO-1114

        commit 9b5c350e870a236c37dfa69af2c33469673c977c
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-27T09:32:44Z

        Removed useless testcase

        commit 76ae262cee268bffacf77fd63c9f17a82d231583
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-27T11:59:33Z

        Re-create validateProperty function

        commit 57cdb09785815295784f5aa846a9ed1b2e3d82e0
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-27T12:02:42Z

        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input.

        commit 60bb94b3559ad7c2c339a9978b87e85c59655703
        Author: Jihun Kang <ykrips@gmail.com>
        Date: 2014-10-28T02:45:26Z

        Add workarounds for a bug on hadItem function of hamcrest-core


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user ykrips opened a pull request: https://github.com/apache/tajo/pull/218 TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. I cannot expect that generic signature of IsCollectionContaining is broken. After the quick review on source code fo IsCollectionContaining, I found the broken point on this class, and add a workaround function on the test case. You can merge this pull request into a Git repository by running: $ git pull https://github.com/ykrips/tajo TAJO-1114 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/218.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #218 commit 412e2e6b8fcd208ded929df100d2ee80ee7a0281 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-17T09:53:13Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit 44b991b43318da6d91758c3ef43232bd71414352 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T06:47:34Z Merge remote-tracking branch 'upstream/master' into TAJO-1114 commit 76720f853dd22b245207d2de8e49fcc9554562f5 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T07:13:01Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit f99e7edcfa51c2955da7a54af4a2edf7b11c6068 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T07:13:53Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit 0ad448d1e51a35778b527a952dbd2c36345579e5 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T07:15:00Z Merge remote-tracking branch 'upstream/master' into TAJO-1114 commit ce0fb6d44e5782c28618b735d95b10c57d3f4692 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T12:47:27Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit 3f3a23a138cc523316cd2b28f1251921a55d404e Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T13:02:30Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit b953102a48bb75136172a584d95191273a386903 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-17T09:53:13Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit c510067ce5665a431230e5e56efa574f014bb85d Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T07:13:01Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit 9d666ef75662c36e825b6bcdc1cd22e30d6c59af Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T07:13:53Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit 1b5088bcf515f2e6706be1d1dbec515a9885b8e4 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T12:47:27Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit 7c92470a100732434e3aa453978c5b498eecca6e Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-20T13:02:30Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit b49df082a32d2bb5a987c7444b0b4b0e94fa8836 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-27T08:52:58Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit cc5d128a3a5c2701ed119b7d6de9b8aa81799c08 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-27T09:00:32Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit 5b5fc3995cba4fced25563b35e1966db5a8812d9 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-27T09:10:41Z Removed validate functions from OverridableConf commit d0ee8cd3c52cec0588af53ad8554e40ba437f711 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-27T09:14:09Z Removed unused conf commit 5a9b44b1f6dc6d960721b9a5154cbd33df389882 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-27T09:31:27Z Merge branch ' TAJO-1114 ' of https://github.com/ykrips/tajo into TAJO-1114 commit 9b5c350e870a236c37dfa69af2c33469673c977c Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-27T09:32:44Z Removed useless testcase commit 76ae262cee268bffacf77fd63c9f17a82d231583 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-27T11:59:33Z Re-create validateProperty function commit 57cdb09785815295784f5aa846a9ed1b2e3d82e0 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-27T12:02:42Z TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. commit 60bb94b3559ad7c2c339a9978b87e85c59655703 Author: Jihun Kang <ykrips@gmail.com> Date: 2014-10-28T02:45:26Z Add workarounds for a bug on hadItem function of hamcrest-core
        Hide
        hyunsik Hyunsik Choi added a comment -

        +1

        I also verified the unit test in JVM 1.6. The patch looks nice to me.

        Show
        hyunsik Hyunsik Choi added a comment - +1 I also verified the unit test in JVM 1.6. The patch looks nice to me.
        Hide
        hyunsik Hyunsik Choi added a comment -

        committed it to master branch. Thanks for the quick fix!

        Show
        hyunsik Hyunsik Choi added a comment - committed it to master branch. Thanks for the quick fix!
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

        https://github.com/apache/tajo/pull/218

        Show
        githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/tajo/pull/218
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-build #428 (See https://builds.apache.org/job/Tajo-master-build/428/)
        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input. (Jihun Kang via hyunsik) (hyunsik: rev f728413fe36918a9b5e2381af951c05361900099)

        • tajo-common/src/main/java/org/apache/tajo/validation/PatternValidator.java
        • CHANGES
        • tajo-common/src/main/java/org/apache/tajo/validation/BooleanValidator.java
        • tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java
        • tajo-common/src/main/java/org/apache/tajo/validation/RangeValidator.java
        • tajo-common/src/main/java/org/apache/tajo/util/NumberUtil.java
        • tajo-common/src/main/java/org/apache/tajo/validation/PathValidator.java
        • tajo-common/src/main/java/org/apache/tajo/ConfigKey.java
        • tajo-common/src/main/java/org/apache/tajo/validation/Validator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/JavaStringValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/LengthValidator.java
        • tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java
        • tajo-common/src/main/java/org/apache/tajo/validation/NetworkAddressValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/NotNullValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/AbstractValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/ShellVariableValidator.java
        • tajo-common/src/main/java/org/apache/tajo/SessionVars.java
        • tajo-common/src/main/java/org/apache/tajo/validation/GroupValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/ClassValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/MinValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/MaxValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolationException.java
        • tajo-common/pom.xml
        • tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolation.java
        • tajo-common/src/main/java/org/apache/tajo/validation/Validators.java
        • tajo-core/src/main/java/org/apache/tajo/engine/query/QueryContext.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #428 (See https://builds.apache.org/job/Tajo-master-build/428/ ) TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. (Jihun Kang via hyunsik) (hyunsik: rev f728413fe36918a9b5e2381af951c05361900099) tajo-common/src/main/java/org/apache/tajo/validation/PatternValidator.java CHANGES tajo-common/src/main/java/org/apache/tajo/validation/BooleanValidator.java tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java tajo-common/src/main/java/org/apache/tajo/validation/RangeValidator.java tajo-common/src/main/java/org/apache/tajo/util/NumberUtil.java tajo-common/src/main/java/org/apache/tajo/validation/PathValidator.java tajo-common/src/main/java/org/apache/tajo/ConfigKey.java tajo-common/src/main/java/org/apache/tajo/validation/Validator.java tajo-common/src/main/java/org/apache/tajo/validation/JavaStringValidator.java tajo-common/src/main/java/org/apache/tajo/validation/LengthValidator.java tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java tajo-common/src/main/java/org/apache/tajo/validation/NetworkAddressValidator.java tajo-common/src/main/java/org/apache/tajo/validation/NotNullValidator.java tajo-common/src/main/java/org/apache/tajo/validation/AbstractValidator.java tajo-common/src/main/java/org/apache/tajo/validation/ShellVariableValidator.java tajo-common/src/main/java/org/apache/tajo/SessionVars.java tajo-common/src/main/java/org/apache/tajo/validation/GroupValidator.java tajo-common/src/main/java/org/apache/tajo/validation/ClassValidator.java tajo-common/src/main/java/org/apache/tajo/validation/MinValidator.java tajo-common/src/main/java/org/apache/tajo/validation/MaxValidator.java tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolationException.java tajo-common/pom.xml tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolation.java tajo-common/src/main/java/org/apache/tajo/validation/Validators.java tajo-core/src/main/java/org/apache/tajo/engine/query/QueryContext.java
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-CODEGEN-build #70 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/70/)
        TAJO-1114: Improve ConfVars (SessionVar) to take a validator interface to check its input. (Jihun Kang via hyunsik) (hyunsik: rev f728413fe36918a9b5e2381af951c05361900099)

        • tajo-common/src/main/java/org/apache/tajo/validation/PathValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/MinValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/GroupValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/ClassValidator.java
        • tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java
        • tajo-common/src/main/java/org/apache/tajo/validation/JavaStringValidator.java
        • tajo-common/src/main/java/org/apache/tajo/util/NumberUtil.java
        • tajo-common/src/main/java/org/apache/tajo/validation/AbstractValidator.java
        • CHANGES
        • tajo-common/pom.xml
        • tajo-common/src/main/java/org/apache/tajo/validation/NotNullValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/Validators.java
        • tajo-common/src/main/java/org/apache/tajo/validation/BooleanValidator.java
        • tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java
        • tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolationException.java
        • tajo-common/src/main/java/org/apache/tajo/validation/LengthValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/PatternValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/MaxValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/NetworkAddressValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/ShellVariableValidator.java
        • tajo-core/src/main/java/org/apache/tajo/engine/query/QueryContext.java
        • tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolation.java
        • tajo-common/src/main/java/org/apache/tajo/SessionVars.java
        • tajo-common/src/main/java/org/apache/tajo/validation/RangeValidator.java
        • tajo-common/src/main/java/org/apache/tajo/validation/Validator.java
        • tajo-common/src/main/java/org/apache/tajo/ConfigKey.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-CODEGEN-build #70 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/70/ ) TAJO-1114 : Improve ConfVars (SessionVar) to take a validator interface to check its input. (Jihun Kang via hyunsik) (hyunsik: rev f728413fe36918a9b5e2381af951c05361900099) tajo-common/src/main/java/org/apache/tajo/validation/PathValidator.java tajo-common/src/main/java/org/apache/tajo/validation/MinValidator.java tajo-common/src/main/java/org/apache/tajo/validation/GroupValidator.java tajo-common/src/main/java/org/apache/tajo/validation/ClassValidator.java tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java tajo-common/src/main/java/org/apache/tajo/validation/JavaStringValidator.java tajo-common/src/main/java/org/apache/tajo/util/NumberUtil.java tajo-common/src/main/java/org/apache/tajo/validation/AbstractValidator.java CHANGES tajo-common/pom.xml tajo-common/src/main/java/org/apache/tajo/validation/NotNullValidator.java tajo-common/src/main/java/org/apache/tajo/validation/Validators.java tajo-common/src/main/java/org/apache/tajo/validation/BooleanValidator.java tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolationException.java tajo-common/src/main/java/org/apache/tajo/validation/LengthValidator.java tajo-common/src/main/java/org/apache/tajo/validation/PatternValidator.java tajo-common/src/main/java/org/apache/tajo/validation/MaxValidator.java tajo-common/src/main/java/org/apache/tajo/validation/NetworkAddressValidator.java tajo-common/src/main/java/org/apache/tajo/validation/ShellVariableValidator.java tajo-core/src/main/java/org/apache/tajo/engine/query/QueryContext.java tajo-common/src/main/java/org/apache/tajo/validation/ConstraintViolation.java tajo-common/src/main/java/org/apache/tajo/SessionVars.java tajo-common/src/main/java/org/apache/tajo/validation/RangeValidator.java tajo-common/src/main/java/org/apache/tajo/validation/Validator.java tajo-common/src/main/java/org/apache/tajo/ConfigKey.java

          People

          • Assignee:
            ykrips Jihun Kang
            Reporter:
            hyunsik Hyunsik Choi
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development