Skip to content

Location resolution through oracle#894

Merged
vkuznecovas merged 6 commits intomasterfrom
location-resolution-through-oracle
Apr 23, 2019
Merged

Location resolution through oracle#894
vkuznecovas merged 6 commits intomasterfrom
location-resolution-through-oracle

Conversation

@vkuznecovas
Copy link
Copy Markdown
Contributor

@vkuznecovas vkuznecovas commented Apr 18, 2019

Updates #863

@vkuznecovas vkuznecovas requested a review from tadovas as a code owner April 18, 2019 13:04
// Resolver allows resolving location by ip
type Resolver interface {
DetectLocation() (Location, error)
ResolveLocation(ip net.IP) (Location, error)
Copy link
Copy Markdown
Member

@soffokl soffokl Apr 18, 2019

Choose a reason for hiding this comment

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

It was kept for cases when a consumer wants to check the provider's IP, whether it's really residential or not.
We, probably, don't have another place for that now.

Comment thread cmd/di.go Outdated
Comment thread cmd/di.go Outdated
@vkuznecovas vkuznecovas force-pushed the location-resolution-through-oracle branch 2 times, most recently from 54d43b8 to 07b4db4 Compare April 23, 2019 07:00
@vkuznecovas vkuznecovas requested review from soffokl and tadaskay April 23, 2019 07:01
Comment thread tequilapi/endpoints/location.go Outdated
@vkuznecovas vkuznecovas force-pushed the location-resolution-through-oracle branch from 4a28393 to 374265c Compare April 23, 2019 07:51
@vkuznecovas vkuznecovas requested a review from soffokl April 23, 2019 08:14
Comment thread cmd/di.go
return fmt.Errorf("unknown location detector type: %s", options.Type)
}

di.LocationResolver, err = location.CreateLocationResolver(di.IPResolver, options.Country, options.City, options.Type, options.NodeType, options.Address, options.ExternalDb, configDirFlag)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We are passing every single option separately, maybe it's time to pass just OptionsLocation?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i'll ahve to ref the node package, which in turn add cyclic dependencies. It's either move the model somewhere, or define a new one here and use that instead. Neither seems to be a proper way of doing things :)

soffokl
soffokl previously approved these changes Apr 23, 2019
@Waldz Waldz force-pushed the location-resolution-through-oracle branch from 374265c to 159d3bf Compare April 23, 2019 08:36
Waldz
Waldz previously approved these changes Apr 23, 2019
@vkuznecovas vkuznecovas force-pushed the location-resolution-through-oracle branch from 159d3bf to 374265c Compare April 23, 2019 08:45
@vkuznecovas vkuznecovas merged commit acb77b9 into master Apr 23, 2019
@vkuznecovas vkuznecovas deleted the location-resolution-through-oracle branch April 23, 2019 09:00
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.

4 participants