@elukey - thanks for bringing this up! I dug into revscoring and mwapi today, it seems that mwapi.Session is interchangeable with requests.Session-- which should do what we want. We can pass a session object and programatically set the headers.

IIUC the two main things we need to change are the hostname and the header (yes?), if so we could do something like this:

internal_url = os.environ.get('wiki_url', 'mw-api.discovery.wmnet')
s = requests.Session()
s.headers.update({'foo': 'bar'})
self.extractor = api.Extractor(
            mwapi.Session(internal_url, user_agent="KFServing revscoring demo", session=s)
        )

Lemme know if this makes any sense, or if I'm missing something

@ACraze wow nice! I checked and the endpoint to use should be api-ro.discovery.wmnet + Host header, the rest should be handled by the mwapi library in theory (so /w/api.php etc..).

We could find a solution that works out of the box for the community users that pull our images from our docker registry (so basically keeping the current behavior) and override WIKI_URL + some-other-env-variable on k8s to be able to create a custom request session. What do you think??

Change 715264 had a related patch set uploaded (by Elukey; author: Elukey):

[machinelearning/liftwing/inference-services@main] editquality: add the WIKI_HOST env variable to model.py

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

Change 715264 merged by Elukey:

[machinelearning/liftwing/inference-services@main] editquality: add the WIKI_HOST env variable to model.py

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

The changes that we did to the editquality image worked! I had to configure (via kubectl) the following in the InferenceService specs:

spec:
  predictor:
    containers:
    - env:
      - name: STORAGE_URI
        value: s3://wmf-ml-models/goodfaith/enwiki/202105140814/
      - name: INFERENCE_NAME
        value: enwiki-goodfaith
      - name: WIKI_URL
        value: https://api-ro.discovery.wmnet
      - name: WIKI_HOST
        value: en.wikipedia.org
      - name: REQUESTS_CA_BUNDLE
        value: /usr/share/ca-certificates/wikimedia/Puppet_Internal_CA.crt
      image: docker-registry.wikimedia.org/wikimedia/machinelearning-liftwing-inference-services-editquality:2021-08-28-101906-production

I think that we should apply this change to the other revscoring model.py files, after that we'll be able to add egress rules to the cluster (limiting connections to swift/s3 and api-ro).

Thanks for the update @elukey, it's great that the changes worked :)

Just a small note - when env variable changes are added to model.py we usually also add them to the inference-service's service.yaml

For example enwiki-goodfaith service.yaml sends the WIKI_URL env variable to its model-server's model.py

In T289778#7317625, @kevinbazira wrote:

Thanks for the update @elukey, it's great that the changes worked :)

Just a small note - when env variable changes are added to model.py we usually also add them to the inference-service's service.yaml

For example enwiki-goodfaith service.yaml sends the WIKI_URL env variable to its model-server's model.py

@kevinbazira ack, will update the service.yaml as well. Is it meant to be for us or for the community? Eventually we will store everything in deployment-charts, but for the moment it makes sense to have a place to store InferenceService configs. I'll try to add comments so that people will know what is optional and what not (also what is tailored for our internal use case and what not). Will send you a code review in a bit :)

Change 715440 had a related patch set uploaded (by Elukey; author: Elukey):

[machinelearning/liftwing/inference-services@main] editquality: update service.yaml after last changes

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

Thank you for the swift changes @elukey, I have +2'd.

Is it meant to be for us or for the community?

This is a pattern we got from the KFServing repo for a custom model-server and it's what we've been using in the development environment. If we keep the configs in the service.yaml it will most likely be easier to onboard community members who reference the KFServing docs as you intuited.

Eventually we will store everything in deployment-charts, but for the moment it makes sense to have a place to store InferenceService configs.

This is good to know. I wasn't aware that the InferenceService configs would end up being stored in the deployment-charts.

Change 715440 merged by Accraze:

[machinelearning/liftwing/inference-services@main] editquality: update service.yaml after last changes

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

@elukey: Nice job! This seems to be working pretty well so far! I noticed the draftquality and topic model servers also call the mw api, should we do the same for those servers as well?

Edit: ah yes, I see you mentioned this already :) i'm happy to jump in and help tack some of those

Change 721414 had a related patch set uploaded (by Accraze; author: Accraze):

[machinelearning/liftwing/inference-services@main] draftquality: use internal mw endpoint

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

Change 721416 had a related patch set uploaded (by Accraze; author: Accraze):

[machinelearning/liftwing/inference-services@main] topic: use internal mw endpoint

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

I've added the two above CRs for draftquality and topic model servers which should cover all of our current revscoring services.

For articlequality, we currently are just feeding text as input to the model, although we may want to add a preprocessing step that fetches article text (see: T285909) which would then need to use internal mw endpoint.

Change 721414 merged by jenkins-bot:

[machinelearning/liftwing/inference-services@main] draftquality: use internal mw endpoint

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

Change 721416 merged by jenkins-bot:

[machinelearning/liftwing/inference-services@main] topic: use internal mw endpoint

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