Add Suggested Edits endpoints #1153

mdholloway · 2019-06-21T15:34:04Z

Adds four new recommendation endpoints:

/data/recommendation/caption/addition/{target}
/data/recommendation/caption/translation/from/{source}/to/{target}
/data/recommendation/description/addition/{target}
/data/recommendation/description/translation/from/{source}/to/{target}

The caption suggestion endpoints are supported only for Commons domains,
and the description suggestion endpoints are supported only for Wikidata
domains. (This is enforced in the service configuration, and is not reflected here.)

Support for commons.wikimedia.org as a default project is added to
config.example.wikimedia.yaml for development and testing.

Support for recommendations endpoints is added to the wikidata.org
project in the example config.

Phab: https://phabricator.wikimedia.org/T224754

mdholloway · 2019-06-21T15:37:33Z

@mateusbs17 This is an example of adding a basic endpoint or set of endpoints to RESTBase with no provision for Cassandra storage.

d00rman

This will need some more work. Namely, end points that have the source and target language cannot go under specific projects, e.g. what is the meaning of source and target if the end point is accessed via en.wp.org? In the same vein, blindly adding all of the recommendation end points to commons doesn't work either. What is the source language for end points that only expect the target to be provided by the client?

TL;DR: these end points need to be separated: one set should go under the global domain (those with both source and target) and another set should go under each project (those for which the target and/or source is known). Better yet, for end points that have both source and target, we should probably rework them so that the source is taken from the domain and then let the client specify the target (that's how it is currently done).

thesocialdev · 2019-06-25T00:44:29Z

@d00rman I'm gonna try to answer these questions.

[...] e.g. what is the meaning of source and target if the end point is accessed via en.wp.org?

It will return 400 - Unsupported domain

The allowed domains are:

description_allowed_domains:
        - www.wikidata.org
        - test.wikidata.org
        - wikidata.beta.wmflabs.org
caption_allowed_domains:
        - commons.wikimedia.org
        - test-commons.wikimedia.org
        - commons.wikimedia.beta.wmflabs.org

What is the source language for end points that only expect the target to be provided by the client?

I might be wrong, but the addition endpoints only need to know the target language which will get the new description/caption, the source doesn't matter since there is no manipulation of previously existing description/caption.

Better yet, for end points that have both source and target, we should probably rework them so that the source is taken from the domain and then let the client specify the target (that's how it is currently done).

That's something I'm don't know how to answer, probably @joewalsh have a better idea about the service architecture.

I might be missing something, but these endpoints are supposed to work exclusively with commons and there is no need to do language differentiation per client since the users need to have set multiple languages in the app

Pchelolo · 2019-06-25T00:55:47Z

So, I guess we'd need a special project for commons now :(

mdholloway · 2019-06-25T04:45:11Z

There's nothing requiring us to use the wikidata and commons domains per se. We could just treat the domain as the target language, since there's a target language in every case. The source could be a path parameter. (Addition endpoints have no source language.)

IOW, they could look like:

/{target}.wikipedia.org/api/rest_v1/data/recommendation/description/addition
/{target}.wikipedia.org/api/rest_v1/data/recommendation/description/translation/from/{source}

The big drawback to using the wiki language subdomain as the target language is that it means we can't specify a language variant as the target. That would be unfortunate.

The same applies for the caption addition and translation endpoints, though Wikipedia domains would be a more awkward fit in that case, since the results are Commons file pages for files that aren't necessarily used on Wikipedias.

mdholloway · 2019-06-25T04:50:45Z

The reason for preferring wikidata and commons for description and caption suggestions, respectively, is that those are the domains that the results most relate to.

d00rman · 2019-06-25T19:58:13Z

@mdholloway @mateusbs17 FYI, we have merged #1148, so now you can rebase and start working on this.

Adds four new recommendation endpoints: - /data/recommendation/caption/addition/{target} - /data/recommendation/caption/translation/from/{source}/to/{target} - /data/recommendation/description/addition/{target} - /data/recommendation/description/translation/from/{source}/to/{target} The caption suggestion endpoints are supported only for Commons domains, and the description suggestion endpoints are supported only for Wikidata domains. Support for commons.wikimedia.org as a default project is added to config.example.wikimedia.yaml for development and testing. Support for recommendations endpoints is added to the wikidata.org project in the example config. Phab: https://phabricator.wikimedia.org/T224754

1- Rebase. 2- Split recommend configurations and add into projects wikidata.wmf and commons.wmf. 3- Create commons.wmf per copy of default.wmf and add recommend-caption entries.

thesocialdev · 2019-06-25T20:27:53Z

@Pchelolo and @d00rman I created v1/recommend-caption.yaml and v1/recommend-description.yaml instead of changing v1/recommend.yaml to add the endpoints to the projects where they are allowed, let me know if you have any concerns.

Pchelolo · 2019-06-25T20:30:08Z

config.frontend.test.yaml

+        paths:
+          /{api:v1}:
+            x-modules:
+              - path: projects/v1/commons.wmf.yaml


The point I've had on the meeting about being able not to create a commons project. We can just use the default project and add the recommned-captions right here in the config

Pchelolo · 2019-06-25T20:32:09Z

The tests/features/specification/monitoring test suite has a list of domains we try out for local x-amples testing. Now that we have endpoints specific to commons and to wikidata, we need to add those domains to the list.

1- Remove commons from projects and add recommend-caption direct in the config file 2- Add wikidata and commons to the monitoring test script as allowed domains

thesocialdev · 2019-06-26T03:15:55Z

I fixed the recommend.yaml but tests still fails. When accessing http://localhost:7231/wikidata.org/v1/?spec, it returns:

{
type: "https://mediawiki.org/wiki/HyperSwitch/errors/server_error",
title: "Site info fetch failed.",
method: "get",
detail: "SiteInfo is unavailable for wikidata.org",
uri: "/wikidata.org/v1/"
}

The last commit removes wikidata.org from the monitoring test just to make CI happy, any thoughts about that?

This depends on the work being done in PR wikimedia#1148. Bug: T223953

d00rman · 2019-06-26T15:42:43Z

@mateusbs17 the domain name is www.wikidata.org, not wikidata.org, that's why the site info fetch fails.

thesocialdev · 2019-06-26T19:10:30Z

@mateusbs17 the domain name is www.wikidata.org, not wikidata.org, that's why the site info fetch fails.

Thanks, that was the problem indeed. Seems fixed, let me know if there is anything else.

Expose new talk endpoint in the wikipedia projects only, no cassandra storage is needed, only regular Varnish layer. The endpoints are: /{domain}/v1/page/talk/{title} /{domain}/v1/page/talk/{title}/{revision} Both return JSON with structured talk page. Bug: T225733

1- Remove cache-control header 2- Add accept-language header 3- Remove unnedeed config

mdholloway · 2019-06-27T15:23:21Z

OK, thanks for the update, @Pchelolo. @mateusbs17 please keep Joe up to date on what's happening with this.

This endpoint won't be cached for now

Release v1.0.0

Pchelolo · 2019-06-27T19:27:11Z

config.fullstack.test.yaml

+                options: '{{options.recommendation}}'
+              - path: projects/proxy.yaml
+                options: *proxy_options
+          /{api:sys}: *default_sys


This portion actually needs to be transferred to the production config. Want me to do it or want me to show you how it's done @mateusbs17 ?

Also, we need to update config.fronted.wikimedia.yaml similarly.

Went ahead and made a prod config change for you https://gerrit.wikimedia.org/r/c/mediawiki/services/restbase/deploy/+/519500

Same question.

Pchelolo · 2019-06-27T19:59:24Z

v1/recommend-caption.yaml

+        - get_from_backend:
+            request:
+              uri: '{{options.host}}/{{domain}}/v1/caption/addition/{target}'
+      x-amples:


Needs x-monitor: true as well

Pchelolo · 2019-06-27T19:59:37Z

v1/recommend-caption.yaml

+        - get_from_backend:
+            request:
+              uri: '{{options.host}}/{{domain}}/v1/caption/translation/from/{source}/to/{target}'
+      x-amples:


Needs x-monitor: true as well

Pchelolo · 2019-06-27T19:59:52Z

v1/recommend-description.yaml

+        - get_from_backend:
+            request:
+              uri: '{{options.host}}/{{domain}}/v1/description/addition/{target}'
+      x-amples:


Needs x-monitor: true as well

Pchelolo · 2019-06-27T20:02:04Z

v1/recommend-description.yaml

+        - get_from_backend:
+            request:
+              uri: '{{options.host}}/{{domain}}/v1/description/translation/from/{source}/to/{target}'
+      x-amples:


Needs x-monitor: true as well.

Actually. while you're at it, could you amend our tests to fail if there IS x-amples and no x-monitoring. Right about here: https://github.com/wikimedia/restbase/blob/master/test/features/specification/monitoring.js#L43

x-monitor is actually not needed if it's value is supposed to be true. The default is x-monitor: true in service-checker

olala... that's the shitty part of having the checker reimplemented in node for testing. out tests need x-monitor

Which config file should we now be using for ordinary testing? These now pass with x-monitor: true when using config.fullstack.test.yaml but only for beta cluster domains. But won't this put beta cluster domains on the Swagger doc page, even in production?

Also, I noticed that most endpoints have x-monitor: false in RESTBase. That's why I did likewise in the new PR before seeing this comment.

… exists.

…sks labs host

thesocialdev · 2019-06-28T00:43:40Z

After adding x-monitor true it was possible to see that the x-amples tests don't work properly, the reason I found is related with the wmflabs host passed as options.

https://recommendation-api-beta.wmflabs.org/www.wikidata.org/v1/description/translation/from/en/to/zh breaks because labs is not able to access wikidata
https://app-editor-tasks.wmflabs.org/www.wikidata.org/v1/description/translation/from/en/to/zh works fine

Even though I added the options to both wikidata and commons, some tests still get 404.

Adding the recommend-caption endpoints in the config file seems is breaking on tests, it seems that the host for appeditortask is ignored, this commit adds it to the default project.

config.frontend.test.yaml

Pchelolo · 2019-07-01T20:52:09Z

projects/v1/default.wmf.yaml

@@ -96,3 +96,5 @@ paths:
      - path: v1/javascript.yaml
        options:
          host: '{{options.mobileapps.host}}'
+      - path: v1/recommend-caption.yaml
+        options: '{{options.appeditortasks}}'


Mmmm... Haven't we decided that this will only be exposed on commons?

Mmmm... Haven't we decided that this will only be exposed on commons?

Er, that's news to me. Was that discussed in the sync meeting last week? The description suggestion endpoints don't make sense on Commons.

Never mind, misunderstood the comment ;)

Pchelolo · 2019-07-01T21:01:46Z

As for x-amples... hm... Would the labs instance support beta commons and beta wikidata? You can expose those under beta domains and not specify domain in the test explicitly - that should work

and pass default options to x-modules.

Expose new talk endpoint

Addressed.

Pchelolo · 2019-07-01T21:42:20Z

config.frontend.test.yaml

@@ -29,6 +29,8 @@ default_project: &default_project
                    host: https://citoid-beta.wmflabs.org
                  recommendation:
                    host: https://recommendation-api-beta.wmflabs.org
+                  appeditortasks:
+                    host: https://app-editor-tasks.wmflabs.org


What is this domain pointing too? it's a different instance of recommendation api service or what is this?

And if so, why are we pointing this to a different domain here?

That's the labs instance @mdholloway was working on. I think this instance has a special setup for the endpoints to work with Extension:WikimediaEditorTasks, commons and wikidata.

Hm. but where would we point production too? To recommendation-api LVS? I think we need a bit more info from @mdholloway here.

@Pchelolo, see #1153 (comment)

Hm. but where would we point production too? To recommendation-api LVS? I think we need a bit more info from @mdholloway here.

Yes, the deployment followed the regular recommendation-api target machines. The limitations of the beta-cluster that made us create a new instance to perform the actions the Apps team needed for testing.

I'm not entirely sure why the normal recommendation-api service can not handle these recommendations.. Since the normal recommendation-api.beta.wmflabs.org is as well completely under our control, why can't we make that one work and not create additional installations of the same thing?

Perhas cause the recommendation service installed in beta doesn't seem to work. Then we need to fix it and not create a separate installation.

This should indeed point to the Beta Cluster. app-editor-tasks was just for prototyping and testing, particularly before the code was merged onto recommendation-api master.

(Pointing to app-editor-tasks in this PR rather than BC like the others was probably just an error on my part.)

Ah, OK, I see what's going on here. Yeah, we definitely don't want to introduce an appeditortasks service to RB. I'll update the PR.

thesocialdev · 2019-07-01T21:42:44Z

As for x-amples... hm... Would the labs instance support beta commons and beta wikidata? You can expose those under beta domains and not specify domain in the test explicitly - that should work

That's something @mdholloway would know more about. ~~I can't find the documented thing (e-mail/wiki page) that explains this labs setup.~~

EDIT: Extension:WikimediaEditorTasks/Testing_environment_setup

mdholloway · 2019-07-02T19:39:04Z

I'm not able to force-push my updates, so I opened a new PR at #1159.

into suggested-edits

This is just an early-stage dev/testing instance. All testing here should be performed against Recommendation API in the Beta Cluster.

Pchelolo · 2019-07-03T12:37:40Z

ok, now we have 2 of these and also something clearly went wrong with git history here... we definitely can't merge this with broken history.

mdholloway · 2019-07-03T13:37:47Z

Sorry for the mess. Cleaned up and rebased in #1160.

@thesocialdev You're not configured as a reviewer in this repo, so I can't add you, but your reviews are also welcome.

mdholloway requested review from Pchelolo and d00rman June 21, 2019 15:34

mdholloway force-pushed the suggested-edits branch from 3288e19 to 157483f Compare June 21, 2019 15:35

mdholloway requested a review from berndsi June 21, 2019 15:36

mdholloway force-pushed the suggested-edits branch from 157483f to 3f4f8be Compare June 21, 2019 15:53

d00rman previously requested changes Jun 24, 2019

View reviewed changes

d00rman added the API label Jun 24, 2019

mdholloway changed the title ~~Add Suggested Edits endpoints to Recommendations~~ Add Suggested Edits endpoints Jun 25, 2019

mdholloway added 2 commits June 25, 2019 17:14

Rebase and add recommend to wikidata and commons

314688c

1- Rebase. 2- Split recommend configurations and add into projects wikidata.wmf and commons.wmf. 3- Create commons.wmf per copy of default.wmf and add recommend-caption entries.

thesocialdev force-pushed the suggested-edits branch from 3f4f8be to 314688c Compare June 25, 2019 20:24

Release v1.0.0

1c2d9f5

Pchelolo reviewed Jun 25, 2019

View reviewed changes

thesocialdev added 2 commits June 25, 2019 18:21

Remove commons from projects

33967f2

1- Remove commons from projects and add recommend-caption direct in the config file 2- Add wikidata and commons to the monitoring test script as allowed domains

Fix test errors

6a689c3

Build: Add the Blubber config file

d43da91

This depends on the work being done in PR wikimedia#1148. Bug: T223953

Fix wikidata url and add it to config.frontend and monitoring tests

ea78783

thesocialdev added 2 commits June 26, 2019 16:45

Fix talk endpoint configs

d64ae4b

1- Remove cache-control header 2- Add accept-language header 3- Remove unnedeed config

thesocialdev and others added 2 commits June 27, 2019 13:57

Remove vary and cache_control option

6504458

This endpoint won't be cached for now

Merge pull request wikimedia#1155 from d00rman/rel/v1.0.0

373350e

Release v1.0.0

Pchelolo reviewed Jun 27, 2019

View reviewed changes

berndsi and others added 5 commits June 27, 2019 15:27

talk: drop CSP, x-CPS, x-frame-options from x-amples

248f5e2

talk: add x-monitor

645740a

Fix x-request-handler and add x-monitor false to the second endpoint

cafd80d

Fix x-monitor identation. Check for undefined x-monitor when x-amples…

65612a0

… exists.

Add x-monitor true for the recommendation endpoints and app-editor-ta…

a945edb

…sks labs host

Add recomend-caption to default project

59b6b54

Adding the recommend-caption endpoints in the config file seems is breaking on tests, it seems that the host for appeditortask is ignored, this commit adds it to the default project.

Pchelolo reviewed Jul 1, 2019

View reviewed changes

config.frontend.test.yaml Show resolved Hide resolved

Pchelolo reviewed Jul 1, 2019

View reviewed changes

thesocialdev and others added 2 commits July 1, 2019 18:14

Remove recommend-caption from default project

70e7ffc

and pass default options to x-modules.

Merge pull request wikimedia#1156 from thesocialdev/talk_endpoint

ec0d8c4

Expose new talk endpoint

Pchelolo reviewed Jul 1, 2019

View reviewed changes

mdholloway mentioned this pull request Jul 2, 2019

Add suggested edits endpoints, take 2 #1159

Closed

mdholloway added 2 commits July 2, 2019 16:16

Merge branch 'suggested-edits' of https://github.com/mdholloway/restbase

0e6946f

into suggested-edits

Remove references to app-editor-tasks

f596e14

This is just an early-stage dev/testing instance. All testing here should be performed against Recommendation API in the Beta Cluster.

mdholloway closed this Jul 3, 2019

Add Suggested Edits endpoints #1153

Add Suggested Edits endpoints #1153

Conversation

mdholloway commented Jun 21, 2019 • edited Loading

mdholloway commented Jun 21, 2019

d00rman left a comment

Choose a reason for hiding this comment

thesocialdev commented Jun 25, 2019 • edited Loading

Pchelolo commented Jun 25, 2019

mdholloway commented Jun 25, 2019 • edited Loading

mdholloway commented Jun 25, 2019

d00rman commented Jun 25, 2019

thesocialdev commented Jun 25, 2019

Choose a reason for hiding this comment

Pchelolo commented Jun 25, 2019

thesocialdev commented Jun 26, 2019

d00rman commented Jun 26, 2019

thesocialdev commented Jun 26, 2019

mdholloway commented Jun 27, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d00rman Jun 27, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thesocialdev commented Jun 28, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pchelolo commented Jul 1, 2019

Pchelolo Jul 1, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thesocialdev Jul 1, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thesocialdev commented Jul 1, 2019 • edited Loading

mdholloway commented Jul 2, 2019

Pchelolo commented Jul 3, 2019

mdholloway commented Jul 3, 2019

mdholloway commented Jun 21, 2019 •

edited

Loading

thesocialdev commented Jun 25, 2019 •

edited

Loading

mdholloway commented Jun 25, 2019 •

edited

Loading

d00rman Jun 27, 2019 •

edited

Loading

thesocialdev commented Jun 28, 2019 •

edited

Loading

Pchelolo Jul 1, 2019 •

edited

Loading

thesocialdev Jul 1, 2019 •

edited

Loading

thesocialdev commented Jul 1, 2019 •

edited

Loading