[GitHub] tajo pull request: TAJO-2156: Create GeoIP functions taking variou...

[eminency]

Github user eminency commented on the pull request:

https://github.com/apache/tajo/pull/1027#issuecomment-222081661
  
Separated issues and refined code.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tajo pull request: TAJO-2156: Create GeoIP functions taking variou...

[eminency]

Github user eminency commented on the pull request:

https://github.com/apache/tajo/pull/1027#issuecomment-220504281
  
Good suggestion. I will separate it. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tajo pull request: TAJO-2156: Create GeoIP functions taking variou...

[jinossy]

Github user jinossy commented on the pull request:

https://github.com/apache/tajo/pull/1027#issuecomment-220500912
  
@eminency 
Geoip2 seems to need to add a mock DataReader.
If we add a geoip2 function by new jira issue, It would be better. What do 
you think?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tajo pull request: TAJO-2156: Create GeoIP functions taking variou...

[eminency]

Github user eminency commented on the pull request:

https://github.com/apache/tajo/pull/1027#issuecomment-219928860
  
Oh, I got it.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tajo pull request: TAJO-2156: Create GeoIP functions taking variou...

[jinossy]

Github user jinossy commented on the pull request:

https://github.com/apache/tajo/pull/1027#issuecomment-219928681
  
The Initial code did not add a tests. because geoip is GPL.
Now, geoip function is breaking changes. we should add a tests.
GeoIP2 is Apache 2.0 license. so we can add the module and tests in tajo 
binary


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tajo pull request: TAJO-2156: Create GeoIP functions taking variou...

[eminency]

Github user eminency commented on the pull request:

https://github.com/apache/tajo/pull/1027#issuecomment-219926775
  
Hi, @jinossy 

Other geoip functions(old ones) have not had a test.
I think it is because of license problem - GeoIP library is set as 
'provided' in pom.xml, so test code doesn't work.

Is that wrong?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tajo pull request: TAJO-2156: Create GeoIP functions taking variou...

[jinossy]

Github user jinossy commented on the pull request:

https://github.com/apache/tajo/pull/1027#issuecomment-219903294
  
This changes should be added tests.
If you change GeoIP to GeoIP2, it would be better for tests.
please see the lincese and csv database for testing

http://maxmind.github.io/GeoIP2-java
https://dev.maxmind.com/geoip/geoip2/geoip2-city-country-csv-databases


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tajo pull request: TAJO-2156: Create GeoIP functions taking variou...

[eminency]

GitHub user eminency opened a pull request:

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

TAJO-2156: Create GeoIP functions taking various types instead of INET4 
type 



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

$ git pull https://github.com/eminency/tajo geoip_funcs

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

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


commit 300f78964e475f1ba728d6a614379cbafef09ce1
Author: Jongyoung Park 
Date:   2016-05-16T07:39:09Z

new geoip functions are added




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---