Move Routing/AOIs/geocoding provider config to central

saleiva commented 8 years ago

cc/ @rafatower @javisantana

rafatower commented 8 years ago

Do we want to have it at user/org level?

As per our discussion with rochoa camshaft should be agnostic with respect to the provider, just accept a parameter, so it's the builder itself who should take that decision. Although we do that for geocodingI think it is also good to keep the dataservices api ignorant of that.

javisantana commented 8 years ago

why are we talking here about camshaft?

rafatower commented 8 years ago

because of this https://github.com/CartoDB/camshaft/pull/147#issuecomment-233299984

saleiva commented 8 years ago

At user level, yeah

2016-07-19 17:35 GMT+02:00 Rafa de la Torre notifications@github.com:

because of this CartoDB/camshaft#147 (comment) https://github.com/CartoDB/camshaft/pull/147#issuecomment-233299984

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/CartoDB/cartodb/issues/8989#issuecomment-233672038, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIENQS3MI-cmYd11HvK60ePWSZoJ_PXks5qXO6pgaJpZM4JP00Y .

Sergio Alvarez Leiva

javisantana commented 8 years ago

wait, there are two providers here:

1) the provider that dataservices api uses when we call geocoding/whatever methods 2) the provider we use in the analysis

Should they be the same?

saleiva commented 8 years ago

I think yes.

2016-07-19 19:12 GMT+02:00 javi santana notifications@github.com:

wait, there are two providers here:

1) the provider that dataservices api uses when we call geocoding/whatever methods 2) the provider we use in the analysis

Should they be the same?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/CartoDB/cartodb/issues/8989#issuecomment-233701684, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIENSt0QyW9civ5kBnDAz4D-8UTwwdLks5qXQV7gaJpZM4JP00Y .

Sergio Alvarez Leiva

javisantana commented 8 years ago

the we should add to camshaft a "default" provider so the default is used (and that default should be the default)

rafatower commented 8 years ago

kind of related to https://github.com/CartoDB/cartodb/pull/8872

to sum up, as in other services:

provider config for AOI's should be modifiable through central, at user/org level
the boxes will have such config replicated, and send it from the builder to the analysis layer
the analysis layer already has a default provider (no provider param means "use analysis default"), no changes needed there
the dataservices layer has "generic" methods that go to here and specific methods that go to mapzen. Similar to what we've done for geocoders, they can all be split into specific methods https://github.com/CartoDB/dataservices-api/issues/209

the first 2 points are the important ones. the last point can be deferred.

Update: It's not only AOI's but also routing and geocoding and to be honest I don't know how it is done for those at the analysis level

javisantana commented 8 years ago

the boxes will have such config replicated, and send it from the builder to the analysis layer

No, camshaft should use the generic method by default

the analysis layer already has a default provider (no provider param means "use analysis default"), no changes needed there

Not true -> https://github.com/CartoDB/camshaft/blob/master/lib/node/nodes/trade-area.js#L63

ethervoid commented 8 years ago

Ok to summarize what I'm doing for this task:

Add geocoder_provider, isolines_provider and routing_provider fields in central and box for users and organizations
Add this fields to the user and organizations decorator, this way the builder could get them with the rest of user/org data and use it as the provider
The fields could be null and we can avoid to update all the users but we're on time to create the DB columns with a default provider.
I'm adding this fields to the redis metadata too so could be used by the dataservices_api in the generic functions but this should be removed and leave only the specific functions to avoid confusion.

What do you think? any complaints?

javisantana commented 8 years ago

Add this fields to the user and organizations decorator, this way the builder could get them with the rest of user/org data and use it as the provider

the frontend should NOT know about the provider in order to send it to the analysis. Camshaft should use the default dataservices function which is configured to use the right provider

ethervoid commented 8 years ago

Ok, I like that idea :)

One question, in case we have an organization user with different providers for organization and user which one prevails?

ethervoid commented 8 years ago

Another question:

If we move the provider logic to dataservices-api through the config that the user/org have in box, we need to define which default values we're going to have for all the providers.

Please @saleiva answer which default providers we're going to have for geocoder and isolines.

We're going to have this default in dataservices-api server and use it in case the provider is null to avoid make updates to all the users and stuff like that

rafatower commented 8 years ago

the frontend should NOT know about the provider in order to send it to the analysis.

then we need some code in backend to expose the quota of the default service to the frontend, right?

saleiva commented 8 years ago

Default would always be mapzen, alghough until we fix the issues with the isolines, we will set here to all users.

2016-07-20 16:50 GMT+02:00 Rafa de la Torre notifications@github.com:

the frontend should NOT know about the provider in order to send it to the analysis.

then we need some code in backend to expose the quota of the default service to the frontend, right?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CartoDB/cartodb/issues/8989#issuecomment-233972954, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIENTEZJt76USW-AVFhIlPmyCpwxyXSks5qXjWZgaJpZM4JP00Y .

Sergio Alvarez Leiva

ethervoid commented 8 years ago

Ok so I'm going to let the users / orgs have null value in box for the providers and have the default value decided in the dataservices api server which is more easy to make changes and deploy.

So if a user has null value in the provider, dataservices-api decides which provider is the default one. In the current case heremaps for isolines and mapzen for geocoder street

rafatower commented 8 years ago

Agreed.

Then I'd remove the default from camshaft later on, cause we're having too many switch-cases around with "defaults".

javisantana commented 8 years ago

then we need some code in backend to expose the quota of the default service to the frontend, right?

how are we doing that right now? is not like that?

ethervoid commented 8 years ago

RIght now we are exposing the quotas through the user endpoint. There're two points we is not exposed: mapzen isolines and routing because we don't have quota there .

On the other hand we have a key geocoder_quota which is used despite the provider we're using. Now that we have different providers for services this should be fixed in the billing and quota service.

In summary we have:

geocoder_quota that is the same for all the providers
here_isolines_quota just for here isolines service
mapzen routing and isolines doesn't have quota in the user config, we have it in the database server config for all the users.

As you can see this system has grown into a mess so we need to reorganizate it in some way.

CartoDB / cartodb

Move Routing/AOIs/geocoding provider config to central #8989