Here's a schema that will match the response of requests to precache in ORES from ChangeProp: https://gist.github.com/halfak/10183548a4d754935481b9bddf9544e8

Change 357457 had a related patch set uploaded (by Ottomata; owner: Ottomata):
[mediawiki/event-schemas@master] [WIP] Add mediawiki/revision/score schema and topic config

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

@Ottomata just wanted to say thanks for getting this work together! I linked it to the other task where were discussing it.

The Reading team will likely first be using this for the Trending-Service to help understand the quality of the edits. The revision fields are definitely useful for us so we don't have to listen to both events (create and score).

@Jdlrobson do you think the fields in the schema look good for consuming with the trending service?

I'm not sure how I'll be using these yet for trending. I'd need to see what they look like in the real world and how they correlate with "trending news". I assume these will automatically be available in EventStreams events ?

Oh yaaahhh!

When looking at revision fields, note that I removed a bunch of them that were in revision-create. Perhaps instead, it would be worth adding an optional scores field to the revision/create schema, and then re-using the exact same schema in a different topic for this. Maybe we'd call this instead revision-create-score or something. Hmm.

I assume these will automatically be available in EventStreams events ?

Ya!

Perhaps instead, it would be worth adding an optional scores field to the revision/create schema, and then re-using the exact same schema in a different topic for this. Maybe we'd call this instead revision-create-score or something. Hmm.

@Ottomata maybe, that way the fields are just there if needed. If there is no downside, it is probably worthwhile. We had similar thoughts about this when thinking about adding ORES scores this to RESTBase Summaries or Page content responses

I'd need to see what they look like in the real world and how they correlate with "trending news".

Essentially if its implemented as we are discussing, then you would just consume these events instead of the revision events. They would be the same except have the scores as well.

I think @Jdlrobson is consuming the old format recentchange events, for which we are not really considering adding scores. revision-create is where it's at!

We are using revision-create
See https://github.com/wikimedia/mediawiki-services-trending-edits/blob/master/lib/EventSource.js#L135

Ahhh right, great!

@Halfak, could you look over https://gerrit.wikimedia.org/r/#/c/357457/5/jsonschema/mediawiki/revision/score/1.yaml and see if this makes sense to you?

@Pchelolo, what do we need to do to produce this from changeprop?

@Pchelolo, what do we need to do to produce this from changeprop?

An hour of time to implement it :)

A couple of open questions we need to answer first:

1. Are we going to post all revision-create events to the new stream even if ORES is not supported there or only post events if ORES is there?

2. Right now ChangeProp doesn't go through the EventBus proxy and talks directly to Kafka, so the events we post there are not validated. Do we want these events validated (I think the answer is yes). Do we want ChangeProp to validate the events on it's own against the schemas we now expose via HTTP, or do we want it to go via the proxy in this particular case?

Are we going to post all revision-create events to the new stream even if ORES is not supported there or only post events if ORES is there?

Yeah, hard to say. I guess the depends on what we want to expose in EventStreams, and what to call it. This is kinda why I had considered naming this stream revision-create-score or something.

Will we expose both revision-create and revision-score? If revision-score doesn't have all revision create events in it, then it will be a little annoying if your use case is: give me all revisions, and if possible scores too.

I think the most useful thing would be to include all revision-create events, even if there is no score.

Do we want ChangeProp to validate the events on it's own against the schemas we now expose via HTTP, or do we want it to go via the proxy in this particular case?

Not sure! If change-prop does validation of the events, I think I'm fine with that.

Not sure! If change-prop does validation of the events, I think I'm fine with that.

It doesn't do it right now, but it's easy enough to add that feature.

Not sure! If change-prop does validation of the events, I think I'm fine with that.

Hm... looking at that more I'm not sure as well... Feels like if we add verification functionality to Change-Prop, we're duplicating what we already have in the proxy, but avoid some significant load on the proxy.. hm...

In T167180#3371757, @Ottomata wrote:

Are we going to post all revision-create events to the new stream even if ORES is not supported there or only post events if ORES is there?

Yeah, hard to say. I guess the depends on what we want to expose in EventStreams, and what to call it. This is kinda why I had considered naming this stream revision-create-score or something.

Will we expose both revision-create and revision-score? If revision-score doesn't have all revision create events in it, then it will be a little annoying if your use case is: give me all revisions, and if possible scores too.

I think the most useful thing would be to include all revision-create events, even if there is no score.

This sounds like we are pushing this forward without a clear understanding of what we want to get out of it. Personally, I think it would be better to expose only events if/when a score for a revision is calculated; re-emitting the same event under a different name seems like a bad pattern to me, doesn't it?

In T167180#3371947, @Pchelolo wrote:

Not sure! If change-prop does validation of the events, I think I'm fine with that.

Hm... looking at that more I'm not sure as well... Feels like if we add verification functionality to Change-Prop, we're duplicating what we already have in the proxy, but avoid some significant load on the proxy.. hm...

That. Let's not duplicate if we don't have to. The revision-create traffic is pretty low (a couple dozen events per second), so that should not have an impact on the HTTP service.

That. Let's not duplicate if we don't have to. The revision-create traffic is pretty low (a couple dozen events per second), so that should not have an impact on the HTTP service.

It would be nice to make it identical how we produce all events. So either all go directly to Kafka, or all go via the proxy..

This sounds like we are pushing this forward without a clear understanding of what we want to get out of it. Personally, I think it would be better to expose only events if/when a score for a revision is calculated; re-emitting the same event under a different name seems like a bad pattern to me, doesn't it?

We don't emit revision-create events in EventStreams at all right now, but I agree that to avoid confusion, we won't call the revision-score event /v2/stream/revision-create in EventStreams. That's why I was originally re-using the revision/create schema with an optional score event, so that this stream could be 'revision-create events that have scores where possible'. IIRC, one of the use cases for a stream like this in ERI project was to help build review tools that could consume revision create events, and where possible augment the review process with score information. I don't think we should make people have join events for this, just because some revisions won't have scores.

I think we want to expose a revision-create-with-scores stream publicly. If in Kafka we had topics for revision-create-only-without-scores, and a revision-create-only-with-scores, EventStreams could configure both topics to be part of a /stream/revision-create-including-scores endpoint. But, we don't have a revision-create-only-without-scores topic, so I'm not sure how that would be possible.

Naming the revision-create-with-scores stream will be a little awkward, but I think that it is valuable to have it as a single stream. Adding scores to the revision-create topic seems like the most natural way to do this, no?

It would be nice to make it identical how we produce all events. So either all go directly to Kafka, or all go via the proxy..

K, let's go through eventbus proxy.

We don't emit revision-create events in EventStreams at all right now

We do now! T167670

Any thoughts on if the new score stream should include unscored revision-create events?

Just had a little chat with @Halfak and @Pchelolo in IRC. We've actually discussed this before, back at the MW Dev summit:

https://phabricator.wikimedia.org/T143743#2966929

ReviewStream should be an EventStreams endpoint, e.g. stream.wikimedia.org/v2/stream/editreview (name still TBD), and it should union relevant event topics together. Any relevant event topic that does not yet exist should be created (e.g. log events, revision-score) and be included in this union stream.

So, maybe the revision-score topic should really be all about scores, and not so much of a duplicate of revision-create + scores. We'd then expose the following streams in EventStreams:

• revision-create (all about revision creation)
• revision-score (all about ORES scores)
• editreview (name TBD, union event stream of revision-create + revision-score + other useful thing topics)

Then we can sidestep the argument about a duplicate stream endpoint revision-create-including-scores, since we will have one that should be useful enough for review tool builders.

Yes, that's what I had in mind should happen here. At some point in the discussion I think we started mixing EventBus and EventStream and the discussion took a different turn :)

Yeah, time erases my memory. Good thing we have phabricator!

Ok, I'll take a pass at the revision-score schema and see how it goes. Maybe it'll just be real simple.

Ok, in https://gerrit.wikimedia.org/r/#/c/357457/8 I removed a lot of extra revision metadata info. I've left some essential stuff, but this makes a revision-score topic much more about the scores, and doesn't look so redundant when exposed along with a revision-create topic in a unioned revision-review (TBD) stream in EventStreams.

@mobrovac One of the things we want to be able to do with the new event is expose ORES scores in summaries: T157132: Add ORES articlequality data to summaries? .

At a previous Services sync I think someone mentioned that having to clear the cache on both the revision-create event AND the revision-score event would be bad because that means you have to update the cache twice for each revision. But it seems that isn't an issue anymore?

In T167180#3425375, @Fjalapeno wrote:

@mobrovac One of the things we want to be able to do with the new event is expose ORES scores in summaries: T157132: Add ORES articlequality data to summaries? .

At a previous Services sync I think someone mentioned that having to clear the cache on both the revision-create event AND the revision-score event would be bad because that means you have to update the cache twice for each revision. But it seems that isn't an issue anymore?

We will still want to re-render the summary as soon as the update is available instead of waiting for ORES to calculate the scores. Once the scores are available, we can update the summary once again (or have RB call ORES if scoring data is not available at the time of retrieval).

@mobrovac cool… sounds good to me 👍

Hey yall, I just bike shed revision-score event schema with @Halfak for a while, we had some insights.

First of all, in https://gerrit.wikimedia.org/r/#/c/357457/9/jsonschema/mediawiki/revision/score/1.yaml, I currently have things like

prediction:
         description: Not all models have predictions, but when they do, they could be anything!
         type:
           - number
           - string
           - boolean
           - array
           - object

and

# A score model can create data with any schema it likes,
# so we allow any additional fields here.  ¯\_(ツ)_/¯
# (additionalProperties is true by default, setting here here just to be explicit.)
additionalProperties: true

This is going to be pretty annoying for downstreams consumers (especially if you try to map this into a relational schema db, like I want to in hive). We can't allow multiple types for the same field.

Instead, we should specify a specific schema for each ORES model. @Halfak actually has (or is working on) having JSONSchemas for each model.

So, we could either:

• A. Add an optional model field for each possible model in this revision-score event, e.g.

scores:
  type: object
  properties?
    damaging:
      prediction:
         type: boolean
      probability:
         false: number,
         true: number
    wp10:
      ...

• B. create event schemas for each model, and emit each as a separate event, e.g.
  • mediawiki/revision/score/damaging/1.yaml
  • mediawiki/revision/score/wp10/1.yaml
  • etc.
    
    each with common revision score fields, and then a score object with a schema that is specific to the model.

We could then emit these specific model revision score events to separate topics, and then union them together as a single stream in EventStreams.

@mobrovac, @Pchelolo , thoughts?

Hm, ok, so we are back at discussing work-arounds for not being able to use oneOf and friends. I am strongly voting for option A, since regardless of the number of models, we can always add new fields there. Having multiple events for essentially the same thing is not the way to go (in my mind that amounts to having a different topic/event/schema for each namespace, e.g.).

In order to help consumers, we could potentially also add another property that indicates which models are relevant/used/populated for a given event, something like:

scores:
  type: object
  properties:
    model_in_use:
      type: string
    damaging:
      prediction:
         type: boolean
      probability:
         false: number,
         true: number
      wp10:

Hm, ok, so we are back at discussing work-arounds for not being able to use oneOf and friends.

Why exactly can't use them? If that's because the python validation library doesn't support it, let's just replace the library?

This goes beyond what we can validate in jsonschema. If a field can have multiple types, we can't easily use it in other strongly typed systems. So far, I can work with unknown schemas, but only if the data never has values for the same field with different types. This is actually already causing problems in the page-properties-changed schema, since mediawiki seems to sometimes emit the same property value as a string, other times a boolean, etc.

Ah, I had a tabbing typo in my pseudo-yaml example. Fixed. Option A is not just one 'model_in_use', it would be multiple model schemas as keys in the revision-score scores object. E.g

scores:
  damaging:
    ...
  wp10:
    ...
  ...

Having multiple events for essentially the same thing is not the way to go

The models are not essentially the same thing. Each model is a different type of score for a given revision.

in my mind that amounts to having a different topic/event/schema for each namespace

Agree, this would mean a different event, topic, and schema for each model. revision-score-damaging, revision-score-wp10, etc.

Talked with Marko and Petr, we agreed to do Option A.

@Halfak, let me know when you have the 'model-specific schemas that include the list of possible labels'.

OK! T172566: Include label-specific schemas with model_info

Looks like we'll have that deployed on Sept. 25th

From Aaron: https://ores.wmflabs.org/v2/scores/enwiki/damaging/?model_info=score_schema

Are there plans to expose this in https://stream.wikimedia.org/v2/stream/recentchange ?

Yes! It's just not prioritized, so I only get to work on it when I have a bit of headroom to do so! :)

Wait, in recentchange? No, it will be its own stream.

Moving this to Incoming column, because it appears in the goals for next quarter: https://www.mediawiki.org/w/index.php?title=Wikimedia_Technology/Goals/2017-18_Q4&action=edit&section=11

Great news! Let me know how I can help.

Ok its about time to resurrect this! @Pchelolo, from at https://github.com/wikimedia/change-propagation/blob/master/config.example.wikimedia.yaml#L467, it looks like change-prop is just matching revision-create, and then triggering ORES to score the revision.

How can we get the score? We don't have an event to trigger from ORES once the revision-score has happened, right? Do we need to make ORES send the revision-score event?

@Ottomata, when we send the revision-create event to ORES, precache endpoint we get the scores as a result, but we do not have the capability to inject those results into the event and re-send it in the config. What we could do is to write a js module to handle that.

Hey @Halfak! We are getting back into this. Q: We worked in an error object into the revision-score schema. Petr and I were wondering what the desired behavior is here. Why would we receive an error from /v3/precache, and do we want to even bother emitting a revision-score event if there was an error?

We talked in IRC and I think @Pchelolo and @Ottomata understand this now. The TLDR is that it's possible to have a successful scoring job that cannot generate some scores but it can generate other scores. E.g. edit quality can't be generated when the parent (previous revision) was rev-deleted, but article quality can be generated without the parent.

Change 357457 merged by Ottomata:
[mediawiki/event-schemas@master] Create revision/score schema

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

Mentioned in SAL (#wikimedia-operations) [2018-05-30T13:26:10Z] <ppchelko@tin> Started deploy [changeprop/deploy@43310d4]: Emit revision-score event T167180

Mentioned in SAL (#wikimedia-operations) [2018-05-30T13:27:42Z] <ppchelko@tin> Finished deploy [changeprop/deploy@43310d4]: Emit revision-score event T167180 (duration: 01m 32s)

Mentioned in SAL (#wikimedia-operations) [2018-05-30T13:37:07Z] <ppchelko@tin> Started deploy [eventstreams/deploy@14e0b03]: Recreate config with new puppet and restart service T167180

Mentioned in SAL (#wikimedia-operations) [2018-05-30T13:39:14Z] <ppchelko@tin> Finished deploy [eventstreams/deploy@14e0b03]: Recreate config with new puppet and restart service T167180 (duration: 02m 07s)

Change 436296 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/event-schemas@master] Remove the pattern for request_id from revision-score event.

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

Change 436296 merged by Ottomata:
[mediawiki/event-schemas@master] Remove the pattern for request_id from revision-score event.

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

Due to the very hairy problems in T195979 and T197000, I'm considering removing revision-score from EventStreams and reopening this task as a parent of those two. We shouldn't expose the stream until we are sure of the schema we will use. YARRGHH

FYI, we fixed the hairy problems by supporting map type fields, which the revisions-score stream uses! mediawiki.revision-score is now available in EventStreams!