Page MenuHomePhabricator

Update Recommend tool to adapt Production service
Open, HighPublic

Description

After filing T190014, it seems there is a production service for the tool already in place. We should use it. There is a change in API usage in form of,

/{domain}/v1/translation/articles/{source}{/seed}

CX need to adapt the tool in this format.

Details:

Once this is done, RecommendToolAPIURL can be updated to point Production service.

Details

Related Gerrit Patches:
mediawiki/extensions/ContentTranslation : masterUse production api of recommendation tool

Event Timeline

Pginer-WMF triaged this task as High priority.Mar 19 2018, 11:17 AM
Pginer-WMF moved this task from Backlog to Maintenance backlog on the Language-2018-Jan-Mar board.
Joe added a subscriber: Joe.Mar 20 2018, 6:10 AM

don't use restbase as a middle-man between our internal services, please.

Services should talk to each other directly, not via a router/proxy/cache. We have done this multiple times here in the last 4 years, and it is an antipattern. Going through restbase for service-to-service communication can only achieve the result of introducing another potential layer of errors.

Thanks for the comment, @Joe. Is it still a good idea to directly talk to computationally expensive services? Wouldn't talking to a caching layer be more appropriate then?

Change 420992 had a related patch set uploaded (by Santhosh; owner: Santhosh):
[mediawiki/extensions/ContentTranslation@master] Use production api of recommendation tool

https://gerrit.wikimedia.org/r/420992

Changes:

  • The production api does not accept algorithm, application, search strategy parameters
  • The api can accept only one seed article. This patch uses the latest one from seed articles
santhosh moved this task from Maintenance backlog to In Review on the Language-2018-Jan-Mar board.

don't use restbase as a middle-man between our internal services, please.

We call the API from browsers, not from an internal service or even from PHP side of an extension.

@Pginer-WMF, please not the above changes, especially about seed articles

@Pginer-WMF, please not the above changes, especially about seed articles

Are we using a different seed article when the user clicks "Refresh"? Would it be possible to do so? Should we create a specific ticket?

Are we using a different seed article when the user clicks "Refresh"?

No.

Joe added a comment.Mar 26 2018, 5:13 AM

don't use restbase as a middle-man between our internal services, please.

We call the API from browsers, not from an internal service or even from PHP side of an extension.

Ok, so technically calling the public rest API is the right thing to do.

Joe added a comment.Mar 26 2018, 5:21 AM

Thanks for the comment, @Joe. Is it still a good idea to directly talk to computationally expensive services? Wouldn't talking to a caching layer be more appropriate then?

(I am answering even if my answer doesn't apply in this case - as @Nikerabbit pointed out, the browser calls our public REST API, so it's ok in this case).

Caching might be a good idea, but internally you want to have the individual application to manage its own caching, not to have a service that does (poorly or not) the job of a caching edge proxy.

So your call flow should be:

Service A ==> Service B <==> Service B cache

instead of

Service A ===> Cache <==> Service B

because well, Service B should be able to function even if its caching layer is broken or unavailable (maybe with worse performance).

If you happen to use the same caching service for all of your services, and it also happens to be the main router between different services, you end up creating a huge SPOF, so that today the downtime of any of our services is practically the downtime of RESTbase, plus the downtime of the service itself.

So, it's not advisable to use RESTbase as a mediator for internal calls at all.

API accepts only one seed

What does it mean for the feature? Users get worse suggestions? They always get the same suggestions? Are the other unavailable api parameters affecting anything?

I updated patch https://gerrit.wikimedia.org/r/c/420992/ so that it takes one seed article at a time. Every refresh will take a new seed page. And if it runs out, we wont use seed. If there is no seed, there is a bug for some language pairs, otherwise we get suggestions. T190266: Switch the Recommendation API to use the internal WDQS cluster

I'm moving this to Blocked because we found an issue with Unicode characters in the RESTBase API.

@santhosh I removed Research tag and I'll remain subscribed to this task to get updates on any follow-ups.