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

Description

See: https://codesearch.wmcloud.org/search/?q=VirtualRESTService (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.

Details

	Subject	Repo	Branch	Lines +/-
	Remove use of VirtualRESTService for MW 1.41+	mediawiki/extensions/VEForAll	master	+58 -1

Customize query in gerrit

Related Objects
Search...

Status	Assigned	Task
Resolved	DAlangi_WMF	T338921 Remove VirtualRESTService
Resolved	DAlangi_WMF	T179680 Hard deprecate VirtualRESTService
Resolved	Yaron_Koren	T341131 Replace usage of VRS in non-WMF deployed extensions (case: VEForAll)

Event Timeline

DAlangi_WMF created this task.Jul 5 2023, 12:51 PM

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+

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

gerritbot added a project: Patch-For-Review.Jul 17 2023, 2:15 PM

Change 938679 merged by jenkins-bot:

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

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

Maintenance_bot removed a project: Patch-For-Review.Jul 17 2023, 3:11 PM

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

Yaron_Koren mentioned this in rEVFAebbdbb3cbde8: Remove use of VirtualRESTService for MW 1.41+.Jul 17 2023, 3:50 PM

Yaron_Koren closed this task as Resolved.Jul 17 2023, 4:37 PM

Yaron_Koren claimed this task.

In T341131#9020114, @Yaron_Koren wrote:

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

Thank you @Yaron_Koren for fixing. 🙏🏽

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