Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IPV6 REST geoip created [old] #280

Closed
wants to merge 2 commits into from
Closed

IPV6 REST geoip created [old] #280

wants to merge 2 commits into from

Conversation

DhashS
Copy link

@DhashS DhashS commented May 11, 2014

moved all the unneeded stuff to disabled_stuff

dhash added 2 commits May 11, 2014 18:10
…ip JSON file, New Geoip command, uses API, ipv4/6 and hostnames work
…ip JSON file, New Geoip command, uses API, ipv4/6 and hostnames work
geo = pygeoip.GeoIP(os.path.abspath("./plugins/data/GeoLiteCity.dat"))
if inp[0:7] == 'http://':
inp = inp[7:]
if inp[0:5] == 'www.':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently this will strip www. from any URL. This may change the result, as www.example.com and example.com could resolve to different IP addresses.

Could you explain your reasoning for doing this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, I assumed that socket.gethostbyname would error out on the www. My fault. Please remove this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you should be able to edit it, and push to your fork to add the changes to this pull request. Could you do that?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'll do it in a few hours

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I deleted that fork, so i have to make a new pull, here it is: #282

@daboross
Copy link
Contributor

I think that it really would be preferred if this could be done with an internal process, rather than calling an external API which may or may not continue to function in the future. This does seem like a useful function change though. I think it would be possible to rewrite the current plugin to still use pygeoip, but add support for raw IP addresses and IPv6 however.

@DhashS DhashS changed the title IPV6 REST geoip created IPV6 REST geoip created [old] May 12, 2014
@DhashS DhashS closed this May 12, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants