Page MenuHomePhabricator

Replace usage of VRS in non-WMF deployed extensions (case: VEForAll)
Closed, ResolvedPublic


See: (for non-WMF deployed repos).

NOTE: In the search above, WMF deployed extension are being migrated are being migrated slowly. They're special cases because we need to make use of cache warming for parsoid outputs. This task is about migrating code in the VEForAll extension that uses the VirtualRESTService class to use MultiHttpClient or regular HTTPClient because VRS will be deprecated and removed soon.

See T338643: Replace usage of VirtualRESTService in non-WMF deployed extensions (case: MathSearch) for example.

Acceptance Criteria

  • Replace for VEForAll
NOTE: Once we have replaced usage in these extensions, we can hard deprecate the class and remove in future versions of MW.

Event Timeline

This is good to know. Generally VEForAll tries to copy from the VisualEditor code, but it looks like VE hasn't yet switched over from using VirtualRESTService yet, so I suppose this task is in some way dependent on the relevant VE task, T264669.

Thanks Yaron for diving in. VE recently switched from VRS and is no longer using RESTBase. Everything is now happening in MW core and today, I made a patch that remove VRS related code from VE entirely which is still pending review. See T339227: Remove VRS code from VisualEditor.

VE no longer uses Parsoid via RESTBase - T310376: Make VE Action API use Parsoid directly, hence no longer using the VRS logic.

Oh! Never mind - I didn't realize that was all unused code. I'll have to look into how to implement this, then - it looks like it involves the new ParsoidParserFactory and HtmlTransformFactory classes.

Yeah, you're correct. And also, constructing/accessing the VirtualRESTService using classes like ParsoidVirtualRestService or RestBaseVirtualRestService or related can be replaced by MultiHttpClient instead of using the VRS wrappers. We can straight away use a MultiHttpClient in case we want support for concurrent requests for example.

Change 938679 had a related patch set uploaded (by Yaron Koren; author: Yaron Koren):

[mediawiki/extensions/VEForAll@master] Remove use of VirtualRESTService for MW 1.41+

Change 938679 merged by jenkins-bot:

[mediawiki/extensions/VEForAll@master] Remove use of VirtualRESTService for MW 1.41+

@DAlangi_WMF - thanks for your help with this. I think is fixed now.

Yaron_Koren claimed this task.

@DAlangi_WMF - thanks for your help with this. I think is fixed now.

Thank you @Yaron_Koren for fixing. 🙏🏽