GitHub PullRequestReviewCommentEvent at 2014-01-22 02:50:26
Repository: phoenix
Actor: jtaylor-sfdc
Payload: {"comment":{"url":"https://api.github.com/repos/forcedotcom/phoenix/pulls/comments/9063836","id":9063836,"diff_hunk":"@@ -661,37 +664,46 @@ public IndexBuilder getBuilderForTesting() {\n     return this.builder.getBuilderForTesting();\n   }\n \n-  /**\n-   * Validate that the version and configuration parameters are supported\n-   * @param hBaseVersion current version of HBase on which <tt>this</tt> coprocessor is installed\n-   * @param conf configuration to check for allowed parameters (e.g. WAL Compression only if >=\n-   *          0.94.9)\n-   */\n-  public static String validateVersion(String hBaseVersion, Configuration conf) {\n-    String[] versions = hBaseVersion.split(\"[.]\");\n-    if (versions.length < 3) {\n-      return \"HBase version could not be read, expected three parts, but found: \"\n-          + Arrays.toString(versions);\n-    }\n-  \n-    if (versions[1].equals(\"94\")) {\n-      String pointVersion = versions[2];\n-      //remove -SNAPSHOT if applicable\n-      int snapshot = pointVersion.indexOf(\"-\");\n-      if(snapshot > 0){\n-        pointVersion = pointVersion.substring(0, snapshot);\n-      }\n-      // less than 0.94.9, so we need to check if WAL Compression is enabled\n-      if (Integer.parseInt(pointVersion) < 9) {\n-        if (conf.getBoolean(HConstants.ENABLE_WAL_COMPRESSION, false)) {\n-          return\n-                \"Indexing not supported with WAL Compression for versions of HBase older than 0.94.9 - found version:\"\n-              + Arrays.toString(versions);\n+    /**\n+     * Validate that the version and configuration parameters are supported\n+     * @param hbaseVersion current version of HBase on which <tt>this</tt> coprocessor is installed\n+     * @param conf configuration to check for allowed parameters (e.g. WAL Compression only if >=\n+     *            0.94.9)\n+     * @return <tt>null</tt> if the version is supported, the error message to display otherwise\n+     */\n+    public static String validateVersion(String hbaseVersion, Configuration conf) {\n+        int[] versions = decodeHBaseVersion(hbaseVersion);\n+        if (versions[1] == INDEXING_SUPPORTED_MAJOR_VERSION) {\n+            // less than 0.94.9, so we need to check if WAL Compression is enabled\n+            if (versions[2] < INDEX_WAL_COMPRESSION_MINIMUM_SUPPORTED_VERSION) {\n+                if (conf.getBoolean(HConstants.ENABLE_WAL_COMPRESSION, false)) {\n+                    return \"Indexing not supported with WAL Compression for versions of HBase older than 0.94.9 - found version:\"\n+                            + Arrays.toString(versions);\n+                }\n+            }\n         }\n-      }\n+        return null;\n+    }\n+\n+    public static int[] decodeHBaseVersion(String version) {","path":"phoenix-core/src/main/java/com/salesforce/hbase/index/Indexer.java","position":62,"original_position":62,"commit_id":"dee8fef9e423b967bc8b36a10b857ef4cd29f6f6","original_commit_id":"dee8fef9e423b967bc8b36a10b857ef4cd29f6f6","user":{"login":"jtaylor-sfdc","id":3300897,"avatar_url":"https://gravatar.com/avatar/f5d13f349653ca33e4d8d5fcc38b2c86?d=https%3A%2F%2Fidenticons.github.com%2Fd0109f7b7f834eab29c6d31640cffa32.png&r=x","gravatar_id":"f5d13f349653ca33e4d8d5fcc38b2c86","url":"https://api.github.com/users/jtaylor-sfdc","html_url":"https://github.com/jtaylor-sfdc","followers_url":"https://api.github.com/users/jtaylor-sfdc/followers","following_url":"https://api.github.com/users/jtaylor-sfdc/following{/other_user}","gists_url":"https://api.github.com/users/jtaylor-sfdc/gists{/gist_id}","starred_url":"https://api.github.com/users/jtaylor-sfdc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jtaylor-sfdc/subscriptions","organizations_url":"https://api.github.com/users/jtaylor-sfdc/orgs","repos_url":"https://api.github.com/users/jtaylor-sfdc/repos","events_url":"https://api.github.com/users/jtaylor-sfdc/events{/privacy}","received_events_url":"https://api.github.com/users/jtaylor-sfdc/received_events","type":"User","site_admin":false},"body":"(or if you don't want to depend on that util, perhaps move those to one of your util classes)? It's just be nice to share them and standardize them so that we can find places in the code that are version dependent.","created_at":"2014-01-22T02:50:26Z","updated_at":"2014-01-22T02:50:26Z","html_url":"https://github.com/forcedotcom/phoenix/pull/706#discussion_r9063836","pull_request_url":"https://api.github.com/repos/forcedotcom/phoenix/pulls/706","_links":{"self":{"href":"https://api.github.com/repos/forcedotcom/phoenix/pulls/comments/9063836"},"html":{"href":"https://github.com/forcedotcom/phoenix/pull/706#discussion_r9063836"},"pull_request":{"href":"https://api.github.com/repos/forcedotcom/phoenix/pulls/706"}}}}