You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tajo.apache.org by combineads <gi...@git.apache.org> on 2016/01/19 08:02:18 UTC
[GitHub] tajo pull request: TAJO-2060: Upgrade geoip-api-java library
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 <co...@combineads.co.kr>
Date: 2016-01-19T07:00:41Z
TAJO-2060: Upgrade geoip-api-java library
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2060: Upgrade geoip-api-java library
Posted by jinossy <gi...@git.apache.org>.
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2060: Upgrade geoip-api-java library
Posted by jinossy <gi...@git.apache.org>.
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
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2060: Upgrade geoip-api-java library
Posted by combineads <gi...@git.apache.org>.
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2060: Upgrade geoip-api-java library
Posted by jinossy <gi...@git.apache.org>.
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
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2060: Upgrade geoip-api-java library
Posted by jinossy <gi...@git.apache.org>.
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”.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2060: Upgrade geoip-api-java library
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/tajo/pull/946
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---