We'd like to take a list of articles in the metabook format and generate a single HTML file. The HTML file will be converted to a PDF file using the ElectronRenderService.
@phuedx: See T171964#3552933.
Here's my attempt at answering the questions above. A lot of the decisions depend on choosing the right architecture. Services and Ops teams are best suited to recommend one way or the other.
We identified the changes needed for concatenation in T163272: [Spike] Determine changes necessary for concatenation support . Here is the modified list (as the original spike was about wkhtmltopdf):
Presumably we'll be using Node.js as it's been used across the foundation in various projects and we don't want to introduce yet another stack.
Why not create a shell script and use tools like tr, grep, and wget?
(Also, personally I think Reading Web developers staying unfamiliar with the PHP stack is not a reasonable thing to plan for in the medium term. That lack of familiarity has harmed Reading Web projects in the past and will continue to do so unless addressed.)
tldr: Choice seems to really hinge on who will be looking after this after OCG has been sunset.
chapter/section number is much easier to do via CSS counters than by rewriting the HTML.
Definitely. Support is also pretty good (although wouldn't work in IE6 and 7, but that's fine). This can be done in both a service or php layer and would expect this to come up during implementation.
I don't see the potential benefit in using unix tools.
Agreed. I would rule that out as an option.
I doubt performance is a big deal since PDF rendering will probably take much more time than the simple HTML changes that are proposed here;
Also agree, but we'll probably want to put an upper limit on the amount of pages that can be concatenated, otherwise this could be a vector for attacks regardless of whether done in PHP or service.
(Also, personally I think Reading Web developers staying unfamiliar with the PHP stack is not a reasonable thing to plan for in the medium term. That lack of familiarity has harmed Reading Web projects in the past and will continue to do so unless addressed.)
Given the size of the team and the amount of products we do maintain (including those dumped on us), that we do require knowledge of the PHP stack for I find this quite offensive (and a little personal) to be honest. I'm happy to talk about it outside this phabricator task with management if it would be useful, but I don't think this is the place.
A micro service
Given reading web's projects are becoming more and more Node dependent and services based this is a valid thing to consider. Which team will be maintaining this on the longer term? If reading web, it's going to get a much higher level of support if the code is also services based given that is our Toby and Adam approved strategic direction.
I'll admit that I had a hard time unpacking this comment. The more general point that Readers Web engineers must be familiar with the PHP stack is valid, but I disagree with you that they are unfamiliar with it.
Our medium- and long-term goals all hinge on consuming MediaWiki APIs, some of which RW are on the hook for maintaining. I expect Readers Web to be contributing to whatever stack underlies these APIs whenever the work requires it.
If you haven't already, then please do reach out to @dr0ptp4kt and me with any concerns that you might have and we'll do our best to address them. I think dissecting the outcomes of previous Readers Web projects is way, way out of scope for this task.
That the question as to who maintains this in the long term is open should point us towards maximizing the number of folks (Foundation- and Community-wide) who could help to maintain it IMO. Homing the majority of this system in the PHP stack achieves this. Regardless, we need to be clear about what support we're being expected to provide and what support we can feasibly provide, which is something to @ovasileva and me to consider.
Also, writing the majority of this system in PHP would also leave open the opportunity to integrate the WSExport tool, which powers the "Download as EPUB" link on https://wikisource.org pages.
@bmansurov thank you for diving deep into different stacks to find the best solution that both solves the problem we're facing and meets our requirements as developers.
Traditional Unix tools
As you said, Unix tools are powerful, they are reliable, fast (mostly written in C) and they may be suitable for the current problem. In long-term it will be difficult to maintain/extend the functionality as bash/awk scripts are pretty difficult to work with. As different people said this is off the table
A micro service
Sounds reasonable. I like the idea of being possible to scale. With every month we develop more and more stuff in Node and it works pretty well. I'm worried about two things
I'm also concerned about node being async and the fact that we might have to retrieve 200 articles. Sending all requests asynchronously at the same time might be a weak link. Some requests might fail, what do we do? Probably it's better to send them one after another. Even it takes a bit more time it's safer and we will be able to recover quickly.
Extension:Collection
This sounds good as most of our stuff is written in PHP, and this task (as I mentioned in micro service section) is a perfect candidate for PHP request. It comes in, HTML gets produced and returned. No session/state handling, every request is a unique/standalone job. Using RemexHTML might be useful as it's developed by us, there might be bugs but we're responsible for fixing those, if not there is DOMDocument which does what it needs to do.
I'm just not sure if using Extension:Collection is a proper place as this extension is pretty old (latest version is over 3 years old). While testing it Anthony found many issues (https://phabricator.wikimedia.org/T166565#3320665). Before we start working on Collection extension it will require some refactoring/updating to latest standards. I would prefer to keep Extension:Collection as a UI only as it is mostly a skin Extension. We can re-use same backend for different Skins (Vector, MinervaNueue, Timeless, Cologne) but user-facing stuff might have to be created separately per each skin.
If we decide to add a possibility to create books in Minerva it will require some changes in existing extension (which may break the desktop version). Making a HTML concatenation service as a standalone service/different extension looks more reasonable to me.
It can be a MW extension if we're going to reuse MW stuff, but I don't see a valid reason why we need MediaWiki Core for that.
To me, this is a standalone service that
We would be able to scale this PHP service same as Node service (every request is a new thing, using cache will not help, only spinning new instances of PHP servers or giving more resources to the existing server). Once HTML building is done PHP all memory consumed in given request can be easily freed. The only async thing we can do is downloading data from MW/RB/MCS so it's probably not a big deal - crunching data/traversing dom will take much more time than sending couple calls to the Varnish.
Thanks everyone for a review. I'll highlight a few points that were brought up (please read comments above if you're interested in the details):
Having summarized the above points I'll be bold and suggest that choosing between PHP and Javascript is mostly a convenience issue, it seems. It's a question of whether we want to maintain a new PHP service, or Extension:Collection, or a Node.js serivice and how much up-front work it requires. I personally like @pmiazga 's suggestion with some modifications.
Here's how I see the issue solved.
One way or another we have to create a script that does post-processing as described in T171960. The choice of the script should decide what we do here. @Tgr mentioned some issues with PHP libraries for PDF processing in T168871#3419245. Node.js has PDFKit but I'm not sure how well it suits our needs. We'll need to do a spike and see. On the other hand we've demonstrated that the post-processing can be done using a Python library in T168871#3449625. So, rather than creating a new PHP service, I suggest we bundle concatenation support with post-processing script. We were going to expose the Python script to Extension:Collection anyway, so why not leave Extension:Collection as is and let it talk to the python script, which does concatenation and post-processing? Does anyone see any issues with this suggestion?
We haven't yet spoken about what's required to deploy a separate service. Node.js is attractive in this context as we have direct contact with folk who've deployed such services thus far and the Services team providing support; we get some instrumentation for free, for example. While building out this functionality in PHP in an extension might not be as architecturally clean, we get a lot of support for free at the manageable cost of not being in direct control of the deploy cycle.
I think T171965: [Spike - 8 hours] How should the PDF post-processing script be exposed for use by Extension:Collection might cover this but I think the answers are relevant here.
"We were going to expose the Python script to Extension:Collection anyway, so why not leave Extension:Collection as is and let it talk to the python script, which does concatenation and post-processing".
I noticed work began on this which I wasn't expecting.
Could we pause and talk about this? I didn't know we were using python in the collection extension for example .
I'd appreciate the team having a presentation/ overview of the work and decisions made so far before actually beginning work given the entire team is on the hook for delivering this. I had thought (hoped?) that was the plan with the spike work.
Maybe now is a good time for all the team (or at least Piotr and myself) to get involved and help deliver this...
Sure, I'll put the work on hold. We can talk about this during stand-up. Although an overview/presentation maybe useful for the rest of the team, I encourage you to take a look at the related tasks. We've done a good job of making them clear for outsiders.
As a side-note, T171838: Build out article concatenation according to requirements for books was next in line and we were going to bring the task into the sprint (as we discussed earlier in a meeting). We also talked about this task in another meeting where the team awarded tokens to approve the decision made. That was a signal for me to start working on the task. Also, as a team we haven't been giving enough attention to the OCG replacement tasks, and there are quarterly goals that we need to meet. This fact also urged me to be bold and step up the work.
Do you have any specific concerns about the choice of Python? Or any other aspects of the development?
@Jdlrobson @pmiazga I've created a document to give a rough idea of the things we did so far and of the things that we plan on doing in: https://www.mediawiki.org/wiki/User:Bmansurov_(WMF)/Alternative_way_of_generating_PDF_books
Please take a look and let's continue the conversation in the talk page or the document itself. Feel free to leave your questions on either place and I'll be happy to answer them.
Re: what framework to use for concatenation and/or post-processing:
Re: language used for PDF post-processing:
Nitpicks:
You are not required to do everything at the same time just because it's Node. It's easy to write some kind of worker pool which does at most N requests in parallel. Async fetching seems like an advantage to me (although as you said, not a huge one given this is the fast part). It's not limited to Node though, MediaWiki has async HTTP support via MultiHttpClient (the POC patch uses it to fetch from RESTBase).
if not there is DOMDocument which does what it needs to do.
DOMDocument is not really reliable with HTML5 from what I heard.
Also, while the UI and code quality problems with Extension:Collection don't seem relevant to me (it's not as if it would go away if we do the concatenation elsewhere; the amount to interact with its existing codebase will be the same either way), using the ElectronPdfService extension is always an option. We are talking about functionality that needs to be wrapped around Electron to make it support multi-page documents, so it would make conceptual sense to put it into the extension dedicated to Electron.
Good points, @Tgr. I'll try to reply from the point of view of using Python and why it's a good choice (compared to PHP Extension and a node service).
I wouldn't call the code complete as it's mentioned in the commit message that it contains hacks and the code is a POC patch. Besides, it has a WIP label and hasn't gone through a review processes yet. That said, PHP codebase definitely has a head-start against Python codebase. If we are going to maintain the feature, we have to be careful and not throw code at an extension that hasn't been maintained for such a long time. It will definitely slow us down even in the short run.
- No overhead of setting up and managing a service
Sure, same with Python if we bundle the code with Extension:Collection and call it from PHP (granted running Python code is supported, or we can bundle executables and distribute them with the extension).
- Unblocks Ops to get rid of the ocg boxes / RelEng to get rid of trebuchet / Services to update Node. The concatenation and/or PDF modification logic can always be moved into a service later.
If we are going to move it to a service later, Python script is an ideal stand alone solution. The code will be self-containing, and as you mention above, we already have Python services in our infrastructure, so we won't be introducing anything new in terms of new technologies used when it comes to setting up a new service.
- Can still be exposed as an API if we want clients to be able to build their own UI for it. On one hand it's slightly more convenient because the API has access to the session (where collection data is currently stored), on the other hand using the action API to deliver files will be awkward. Neither of those are big issues though.
I don't think we should be striving for adding more features to the current MediaWiki API when we can create a service that does the same job.
- One Node-based scenario that would make a lot of sense is if we found a good Node library for PDF processing, then we could do everything in Javascript and reduce the number of languages involved in PDF generation. We already have a working solution though and researching PDF libraries is a nontrivial amount of time. (PDFKit at a glance cannot modify existing files so it is not useful here. HummusJS looks more promising.)
I remember us dismissing wkhtmltopdf because it was written in C++ and how it was a security risk. HummusJS also contains C++ code and we can bring up the same arguments here. It will take a long time to get it reviewed too, I imagine.
- I agree the ability to output alternative formats (EPUB, Zim, whatever) is valuable. IMO that's a (weak) argument for doing as much in the extension as possible. We'll probably want a "RESTBase -> concatenated HTML + metadata -> format of choice" pipeline for those as well, but the metadata (and possibly the HTML) will probably need to be subtly different and it's not easy to predict how, so having it travel through more services makes life more difficult. Services are great when there is a simple, stable interface that can serve as a boundary between the different systems. That is the case with Electron (send HTML, get PDF back) but would be much less true for a HTML concatenation or PDF TOC generation service.
I'm not sure I understand why HTML concatenation and PDF post-processing cannot be thought of as similar services to Electron? As I'm currently proposing, we bundle these features into a single Python package, but in the feature, we can definitely move appropriate bits to two different services that do one thing only: concatenation or post-processing.
- Python or Node would be a sensible choice if we find a good library (we did for Pythong already). One thing to consider is whether Node/Python is installed on the appservers. If not, we probably don't want to add it just because of this and might want to look into how well those languages support compiling to an executable.
Good point.
- I would keep the functionality minimal - fewer developers will be familiar with the language (especially if it's Python), and language boundaries are barriers for logging, debugging, refactoring etc. I don't see any advantage in moving HTML concatenation out of the extension (maybe in the long term if we go the Node service route, but not until then).
I think we already have a handful of people who write Python on our team. Also, we'll need to write the PDF post-processing part in Python anyway as things stand. If HTML concatenation is already done in the extension, I'm fine leaving it there, but from my understanding, it's still a WIP. How much work do you think is left to get it done? On the other hand if we can get this functionality done as a standalone script with not much effort now, why not do it now? We'll be saving our feature selves a lot of new work.
The commit message also mentions that the the concatenation code (BookRenderer/RemexCollectionMunger) can be used as-is. The logic for fetching data from Parsoid and MW core (DataProvider) is also easily fixable; I just hardcoded a bunch of URLs because I could not get Electron and RESTBase to work together in Vagrant. The rest of the patch is not really useful, but those already cover the functionality that could be moved to Python.
If we are going to maintain the feature, we have to be careful and not throw code at an extension that hasn't been maintained for such a long time.
I don't see why not. Collection will remain the interface to PDF rendering for the foreseeable future. Some parts of it will need to be touched anyway to integrate the new functionality; the rest doesn't need to be touched either way. But if you really want to use a well-maintained extension, ElectronPdfService is also available (it's currently maintained by WMDE but I imagine they wouldn't be saddened if Readers wanted to take over).
If we are going to move it to a service later, Python script is an ideal stand alone solution. The code will be self-containing, and as you mention above, we already have Python services in our infrastructure, so we won't be introducing anything new in terms of new technologies used when it comes to setting up a new service.
If we plan to run a Python service then it certainly makes sense. This would be the first time an Audiences team would maintain a Python service though, and being familiar with Python and being familiar with running Python web services are two very different things. So, not a decision to make lightly.
I don't think we should be striving for adding more features to the current MediaWiki API when we can create a service that does the same job.
Servicifying things that don't really need to be services is costly. The service will have to be maintained, deploying changes that affect both the service and the extension need to be coordinated, debugging becomes a lot more complicated. Also the extension becomes harder to use for third parties.
I remember us dismissing wkhtmltopdf because it was written in C++ and how it was a security risk. HummusJS also contains C++ code and we can bring up the same arguments here.
Fair point, I didn't notice that. I couldn't find other promising Node libraries with a cursory search.
I'm not sure I understand why HTML concatenation and PDF post-processing cannot be thought of as similar services to Electron?
Electron expects an URL or HTML body and returns a PDF file. That's a very simple and natural interface that's unlikely to change. The proposed service would have to receive a list of RESTBase URLs, title/chapter metadata, author metadata, section metadata... when your interface gets that complex, IMO that's a sign that you don't really need a separate service.
AFAIK we decided to do this in PHP during yesterday's kickoff meeting.
@Tgr is assigned to T171838: Build out article concatenation according to requirements for books to reflect that he's tidying up his WIP change to Extension:Collection, which @bmansurov, @Jdlrobson, and @pmiazga will all help review.
We decided to do this way due to a mix of looming deadline and the reasons laid out in T171964#3547535:
The commit message also mentions that the concatenation code (BookRenderer/RemexCollectionMunger) can be used as-is. The logic for fetching data from Parsoid and MW core (DataProvider) is also easily fixable; I just hardcoded a bunch of URLs because I could not get Electron and RESTBase to work together in Vagrant. The rest of the patch is not really useful, but those already cover the functionality that could be moved to Python.
i.e. that the WIP change isn't actually that far from not being a WIP, e.g. hardcoded URLs need to be moved to configuration variables.
If I've missed a detail, then please feel free to comment and add a link to the comment in the description for posterity. Regardless, this is Done 🎉🎉🎉