Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.11.0
    • Fix Version/s: 0.12.0, 0.11.1
    • Component/s: Function/UDF
    • Labels:
      None

      Description

      Tajo use the dspace-geoip library, version is 1.2.3.
      Now geoip data doesn't work at 1.2.3 version.
      We need change 1.2.14 from 1.2.3 version.

      Below related exception.
      java.lang.ArrayIndexOutOfBoundsException: 254
      at com.maxmind.geoip.LookupService.getCountry(LookupService.java:436)
      at com.maxmind.geoip.LookupService.getCountry(LookupService.java:408)
      at org.apache.tajo.util.GeoIPUtil.getCountryCode(GeoIPUtil.java:44)

        Activity

        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user combineads opened a pull request:

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

        TAJO-2060: Upgrade geoip-api-java library

        Tajo use the dspace-geoip library, version is 1.2.3.
        Now geoip data doesn't work at 1.2.3 version.
        We need change 1.2.14 from 1.2.3 version.

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

        $ git pull https://github.com/combineads/tajo TAJO-2060

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

        https://github.com/apache/tajo/pull/946.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 #946


        commit 89f5304e8e7147640da7c8a683a8d61581021411
        Author: Byunghwa Yun <combine@combineads.co.kr>
        Date: 2016-01-19T07:00:41Z

        TAJO-2060: Upgrade geoip-api-java library


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user combineads opened a pull request: https://github.com/apache/tajo/pull/946 TAJO-2060 : Upgrade geoip-api-java library Tajo use the dspace-geoip library, version is 1.2.3. Now geoip data doesn't work at 1.2.3 version. We need change 1.2.14 from 1.2.3 version. You can merge this pull request into a Git repository by running: $ git pull https://github.com/combineads/tajo TAJO-2060 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/946.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 #946 commit 89f5304e8e7147640da7c8a683a8d61581021411 Author: Byunghwa Yun <combine@combineads.co.kr> Date: 2016-01-19T07:00:41Z TAJO-2060 : Upgrade geoip-api-java library
        Hide
        githubbot ASF GitHub Bot added a comment -

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

        https://github.com/apache/tajo/pull/946#discussion_r50218533

        — Diff: tajo-core-tests/src/test/java/org/apache/tajo/util/TestGeoIPUtil.java —
        @@ -0,0 +1,61 @@
        +/**
        + * Licensed to the Apache Software Foundation (ASF) under one
        + * or more contributor license agreements. See the NOTICE file
        + * distributed with this work for additional information
        + * regarding copyright ownership. The ASF licenses this file
        + * to you under the Apache License, Version 2.0 (the
        + * "License"); you may not use this file except in compliance
        + * with the License. You may obtain a copy of the License at
        + *
        + * http://www.apache.org/licenses/LICENSE-2.0
        + *
        + * Unless required by applicable law or agreed to in writing, software
        + * distributed under the License is distributed on an "AS IS" BASIS,
        + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
        + * See the License for the specific language governing permissions and
        + * limitations under the License.
        + */
        +
        +package org.apache.tajo.util;
        +
        +import static org.junit.Assert.assertNotNull;
        +
        +import org.apache.commons.io.IOUtils;
        +import org.junit.Test;
        +
        +import java.io.File;
        +import java.io.FileOutputStream;
        +import java.io.InputStream;
        +import java.io.OutputStream;
        +import java.lang.reflect.Field;
        +import java.net.URL;
        +import java.util.zip.GZIPInputStream;
        +
        +import com.maxmind.geoip.LookupService;
        +
        +public class TestGeoIPUtil {
        + @Test
        + public void testCountryCode() throws Exception

        { + new GeoIPUtil(); // initialize static field. + + LookupService lookup = new LookupService(getGeoIpData().getPath(), LookupService.GEOIP_MEMORY_CACHE); + System.out.println(GeoIPUtil.class.getDeclaredField("LOG")); + Field lookupField = GeoIPUtil.class.getDeclaredField("lookup"); + lookupField.setAccessible(true); + lookupField.set(null, lookup); + assertNotNull(GeoIPUtil.getCountryCode("154.73.88.82")); + assertNotNull(GeoIPUtil.getCountryCode("154.73.89.143")); + assertNotNull(GeoIPUtil.getCountryCode("154.76.101.156")); + }

        +
        + private File getGeoIpData() throws Exception {
        + File tmpFile = File.createTempFile("GeoIP", ".dat");
        +
        + try (InputStream is = new URL("http://www.maxmind.com/download/geoip/database/GeoLiteCountry/GeoIP.dat.gz")
        + .openConnection().getInputStream();
        + OutputStream out = new FileOutputStream(tmpFile))

        { + IOUtils.copy(new GZIPInputStream(is), out); + }

        + return tmpFile;
        + }
        +}
        — End diff –

        In my opinion, we don’t need to tests of third-party module.

        Show
        githubbot ASF GitHub Bot added a comment - Github user jinossy commented on a diff in the pull request: https://github.com/apache/tajo/pull/946#discussion_r50218533 — Diff: tajo-core-tests/src/test/java/org/apache/tajo/util/TestGeoIPUtil.java — @@ -0,0 +1,61 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.tajo.util; + +import static org.junit.Assert.assertNotNull; + +import org.apache.commons.io.IOUtils; +import org.junit.Test; + +import java.io.File; +import java.io.FileOutputStream; +import java.io.InputStream; +import java.io.OutputStream; +import java.lang.reflect.Field; +import java.net.URL; +import java.util.zip.GZIPInputStream; + +import com.maxmind.geoip.LookupService; + +public class TestGeoIPUtil { + @Test + public void testCountryCode() throws Exception { + new GeoIPUtil(); // initialize static field. + + LookupService lookup = new LookupService(getGeoIpData().getPath(), LookupService.GEOIP_MEMORY_CACHE); + System.out.println(GeoIPUtil.class.getDeclaredField("LOG")); + Field lookupField = GeoIPUtil.class.getDeclaredField("lookup"); + lookupField.setAccessible(true); + lookupField.set(null, lookup); + assertNotNull(GeoIPUtil.getCountryCode("154.73.88.82")); + assertNotNull(GeoIPUtil.getCountryCode("154.73.89.143")); + assertNotNull(GeoIPUtil.getCountryCode("154.76.101.156")); + } + + private File getGeoIpData() throws Exception { + File tmpFile = File.createTempFile("GeoIP", ".dat"); + + try (InputStream is = new URL("http://www.maxmind.com/download/geoip/database/GeoLiteCountry/GeoIP.dat.gz") + .openConnection().getInputStream(); + OutputStream out = new FileOutputStream(tmpFile)) { + IOUtils.copy(new GZIPInputStream(is), out); + } + return tmpFile; + } +} — End diff – In my opinion, we don’t need to tests of third-party module.
        Hide
        githubbot ASF GitHub Bot added a comment -

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

        https://github.com/apache/tajo/pull/946#discussion_r50218535

        — Diff: tajo-core/pom.xml —
        @@ -262,9 +262,9 @@
        <artifactId>commons-codec</artifactId>
        </dependency>
        <dependency>

        • <groupId>org.dspace.dependencies</groupId>
        • <artifactId>dspace-geoip</artifactId>
        • <version>1.2.3</version>
          + <groupId>com.maxmind.geoip</groupId>
          + <artifactId>geoip-api</artifactId>
          + <version>1.2.14</version>
            • End diff –

        Could you change to latest version? and this dependency scope should be changed to “provided”.

        Show
        githubbot ASF GitHub Bot added a comment - Github user jinossy commented on a diff in the pull request: https://github.com/apache/tajo/pull/946#discussion_r50218535 — Diff: tajo-core/pom.xml — @@ -262,9 +262,9 @@ <artifactId>commons-codec</artifactId> </dependency> <dependency> <groupId>org.dspace.dependencies</groupId> <artifactId>dspace-geoip</artifactId> <version>1.2.3</version> + <groupId>com.maxmind.geoip</groupId> + <artifactId>geoip-api</artifactId> + <version>1.2.14</version> End diff – Could you change to latest version? and this dependency scope should be changed to “provided”.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jinossy commented on the pull request:

        https://github.com/apache/tajo/pull/946#issuecomment-173107688

        @combineads
        Thanks for your contribution.
        I left some comments

        Show
        githubbot ASF GitHub Bot added a comment - Github user jinossy commented on the pull request: https://github.com/apache/tajo/pull/946#issuecomment-173107688 @combineads Thanks for your contribution. I left some comments
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user combineads commented on the pull request:

        https://github.com/apache/tajo/pull/946#issuecomment-173843565

        I’ve update the patch that reflects your suggestions. @jinossy
        Thank you for comment.

        Show
        githubbot ASF GitHub Bot added a comment - Github user combineads commented on the pull request: https://github.com/apache/tajo/pull/946#issuecomment-173843565 I’ve update the patch that reflects your suggestions. @jinossy Thank you for comment.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jinossy commented on the pull request:

        https://github.com/apache/tajo/pull/946#issuecomment-173852080

        +1 LGTM!
        Test failure is not related this PR

        Show
        githubbot ASF GitHub Bot added a comment - Github user jinossy commented on the pull request: https://github.com/apache/tajo/pull/946#issuecomment-173852080 +1 LGTM! Test failure is not related this PR
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

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

        Show
        githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/tajo/pull/946
        Hide
        jhkim Jinho Kim added a comment -

        committed it
        Thanks

        Show
        jhkim Jinho Kim added a comment - committed it Thanks
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Tajo-master-CODEGEN-build #661 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/661/)
        TAJO-2060: Upgrade geoip-api-java library. (jhkim: rev 4c7c71f08a9dd2fc7596907a644c8771ccd70923)

        • tajo-core/pom.xml
        • CHANGES
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Tajo-master-CODEGEN-build #661 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/661/ ) TAJO-2060 : Upgrade geoip-api-java library. (jhkim: rev 4c7c71f08a9dd2fc7596907a644c8771ccd70923) tajo-core/pom.xml CHANGES
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-0.11.1-build #157 (See https://builds.apache.org/job/Tajo-0.11.1-build/157/)
        TAJO-2060: Upgrade geoip-api-java library. (jhkim: rev 7610ac75ae76ee9138d2742c6c1ed2d9d012873c)

        • tajo-core/pom.xml
        • CHANGES
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-0.11.1-build #157 (See https://builds.apache.org/job/Tajo-0.11.1-build/157/ ) TAJO-2060 : Upgrade geoip-api-java library. (jhkim: rev 7610ac75ae76ee9138d2742c6c1ed2d9d012873c) tajo-core/pom.xml CHANGES
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-build #1062 (See https://builds.apache.org/job/Tajo-master-build/1062/)
        TAJO-2060: Upgrade geoip-api-java library. (jhkim: rev 4c7c71f08a9dd2fc7596907a644c8771ccd70923)

        • tajo-core/pom.xml
        • CHANGES
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #1062 (See https://builds.apache.org/job/Tajo-master-build/1062/ ) TAJO-2060 : Upgrade geoip-api-java library. (jhkim: rev 4c7c71f08a9dd2fc7596907a644c8771ccd70923) tajo-core/pom.xml CHANGES

          People

          • Assignee:
            combine Byunghwa Yun
            Reporter:
            combine Byunghwa Yun
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Due:
              Created:
              Updated:
              Resolved:

              Development