Skip to content

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
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