Does vagrant need a patch similar to https://gerrit.wikimedia.org/r/233439 to ensure that it uses VRS? Both Visual Editor and Flow should now pick up the Parsoid (or RESTBase) configuration from VRS.
Description
Details
Related Objects
Event Timeline
From IRC:
cscott-free: edsanders, mobrovac: i don't understand vagrant well enough to write the patch. But I believe what's necessary is just to use the VRS config, and probably to explicitly set the domain property (if $wgCanonicalServer isn't working)
mobrovac: the vrs config is there, i think it should just be associated with the ve role, not the restbase one
cscott-free: right, this is exactly the bit of vagrant that I don't understand. ;)
Any updates on this? I can't seem to open VE on Vagrant as of late and I think this might be the culprit.
Change 243147 had a related patch set uploaded (by Mobrovac):
RESTBase: use the domain name as defined by role::mediawiki
TL;DR:
- apply PS 243147
- vagrant roles enable visualeditor restbase && vagrant provision
RESTBase already provides the VRS configuration needed by VE via its role role::restbase. PS 243147 sets its domain correctly so it can be used in conjunction with VE and Parsoid. In order for VE to work in Vagrant, you need to apply that patch to your local MW-Vagrant install and then enable the VE and RESTBase roles with vagrant roles enable visualeditor restbase && vagrant provision.
I tried to replace role::visualeditor's requirements to include RESTBase instead of Parsoid, however, that does not work, because MW-Vagrant does not expose the ports of roles not directly enabled by the user. So we end up with a working RESTBase installation which VE's client-side code cannot reach (because it issues requests to localhost:7231, but that port is not forwarded into the guest VM). This problem is not VE-VRS-RESTBase-Parsoid-specific.
A quicker fix I found worked was to change "VisualEditorParsoidDomain" in extension.json from "localhost" to "127.0.0.1"
That was a problem in the early RESTBase days when it needed beefy Cassandra to back it up. Nowadays it uses the lightweight SQLite back-end in MW-Vagrant, so I don't think that's a real problem (Parsoid needs more resources for parsing than RESTBase needs to route and store requests and their responses).
Yup, but that unfortunately cannot be reliably implemented in MW-Vagrant as it would ask for it to mess with a submodule of its submodule.
I hope the long term fix doesn't involve getting people to enable restbase
That was a problem in the early RESTBase days when it needed beefy Cassandra to back it up. Nowadays it uses the lightweight SQLite back-end in MW-Vagrant, so I don't think that's a real problem (Parsoid needs more resources for parsing than RESTBase needs to route and store requests and their responses).
Performance is important too. Vagrant boxes are running with 1/4 of the system RAM by default, so having another process running because of a configuration issue seems wasteful.
Change 243147 merged by jenkins-bot:
RESTBase: use the domain name as defined by role::mediawiki
If we were talking about HPC systems, I'd agree with you 100%. But in the context where 99% of the performance penalty comes from the overhead of actually having the VM turned on (I'm assuming here that most people that use MW-Vagrant are compelled to use VBox or a similar technology, i.e. cannot use LXC), I don't think an extra quick hop really matters. And keep in mind that's only true for the first time you request the HTML of a page. By using RESTBase subsequent requests for the same revision of a page are actually much faster, even in the VM.
Also, if we really care about the last 40mb of RAM, we could even combine Parsoid & RESTBase in a single process, using service-runner.
This task per se is, hence resolving. However, the user still needs to enable both the visualeditor and restbase roles in order for VE to work properly out of the box. Making it fully functional only by enabling the VE role is a bigger MW-Vagrant role management issue.
Reopening as this can actually be easily dealt with by exposing the ports directly for the visualeditor role.
Change 243530 had a related patch set uploaded (by Mobrovac):
VE: Require RESTBase instead of Parsoid and expose the needed ports
Change 243530 merged by jenkins-bot:
VE: Require RESTBase instead of Parsoid and expose the needed ports