Page MenuHomePhabricator

Review and deploy Score extension to Wikimedia wikis
Closed, ResolvedPublic

Description

Author: sumanah

Description:
http://www.mediawiki.org/wiki/Extension:Score

Review and deploy GrafZahl's Score extension on the cluster. References:

https://bugzilla.wikimedia.org/show_bug.cgi?id=189#c116

"After Nemo's recent mail [1] on wikisource-l, I created a rewrite of the
LilyPond extension at http://www.mediawiki.org/wiki/Extension:Score

Hopefully, this rewrite addresses comments 103 and 105 (sans the "optional"
stuff), and also Tim Starling's recent review [2].

[1] http://lists.wikimedia.org/pipermail/wikisource-l/2011-December/001053.html
[2] https://www.mediawiki.org/wiki/Special:Code/MediaWiki/98414#c27299 "


Version: unspecified
Severity: enhancement

Details

Reference
bz33193

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 12:07 AM
bzimport set Reference to bz33193.

Commenting here to create comment links (hopefully):

Bug 189 comment 103 -- Uses lilypond, so I think this is addressed.
Bug 189 comment 105 -- ditto

I made some minor fix ups to the extension in r106499 (mostly fixes relating to which wfMsg function needed to be used, see the commit summary for full details)

I wonder if the alt text should also be the title text in "Raw" mode.

I think this extension might need to be changed to work with the new file backend stuff. (I haven't actually read the new file backend code, so couldn't say for sure).

Anyhow, good work on getting this moving forward

Graf.Zahl wrote:

(In reply to comment #2)

I think this extension might need to be changed to work with the new file
backend stuff. (I haven't actually read the new file backend code, so couldn't
say for sure).

What precisely is the "new file backend stuff"? I found http://www.mediawiki.org/wiki/FileBackend, but this appears to be still in the planning stage?

(In reply to comment #3)

(In reply to comment #2)

I think this extension might need to be changed to work with the new file
backend stuff. (I haven't actually read the new file backend code, so couldn't
say for sure).

What precisely is the "new file backend stuff"? I found
http://www.mediawiki.org/wiki/FileBackend, but this appears to be still in the
planning stage?

Well its no longer in the planning stage as of yesterday (r106752). At this stage there's probably not to much docs for it, so I wouldn't be super concerned just yet.

-shell

Not required until extension is fully reviewed and has been marked ok for deployment

sumanah wrote:

GrafZahl, thanks for your work and sorry for the wait. I see via https://www.mediawiki.org/w/index.php?title=Special:Code/MediaWiki&path=%2Ftrunk%2Fextensions%2FScore%2F that you have made a few more improvements since December. Is your extension ready for a thorough review for deployment? If so, please use https://www.mediawiki.org/wiki/Git/Conversion/Extensions_queue to request a move to Git. It needs to be in Git before it can be deployed on WMF sites. Thanks!

I've fixed up a few minor issues with it, and added it to the git migration queue.

The main remaining problem with it is the high load it will put on the NFS server. The Math extension has a DB table which tracks created files, and that allows the stat() syscalls to be avoided in favour of slave DB requests. By contrast, this extension will do several stat() calls per score on every parse.

Storing the source text and options in the database would allow the images to be regenerated after a software update, a feature sorely missed in the Math extension.

Generating the MIDI file unconditionally would allow for more effective caching of the LilyPond output, and would avoid the need to separately check whether the MIDI file exists on every cache hit.

sumanah wrote:

GrafZahl, can you address this performance issue this week? If not, Tim is interested in working on it at the end of the week (Friday in Australia). Thanks.

beau wrote:

Is anyone working on this? If not, I can try to adddress issues raised by Tim.

(In reply to comment #9)

Is anyone working on this? If not, I can try to adddress issues raised by Tim.

That would be awesome! I haven't heard from upstream in a while, so if you can take this, we'll get it deployed that much sooner.

beau wrote:

I have submitted 4 changes for review:

  • gerrit change #7359
  • gerrit change #7378
  • gerrit change #7492
  • gerrit change #7581

beau wrote:

Still no review for the code I made.

What's the status of this bug? Are Beau's changes still awaiting review?

I think so. The changes on Sumana's link were not reviewed.

sumanah wrote:

Adding Arthur Richards to cc since he's done some review for one of the outstanding Score changesets. Arthur, I have taken the liberty of also requesting you to review the other outstanding patchsets for Score, in case you have time?

Markus, adding you to cc because of your Wikimania presentation https://wikimania2012.wikimedia.org/wiki/Submissions/We_want_scores! -- couldn't find Anja's email in the BZ user list, but maybe she'd like to be cc'd as well?

Thanks Sumana. I'm happy to keep doing review, which will generally happen on Fridays although I sometimes find other times during the week to get some review done. I think this is a super cool extension and would definitely like to see it eventually out in the wild :)

Thanks Sumana! I added Anja to cc.

@Arthur, let me know if I can be of any help.

Any progress, a month on? It all sounds rather exciting.

sumanah wrote:

We're awaiting fixes in response to code review by Tim Starling, Markus Glaser, and Arthur Richards. https://gerrit.wikimedia.org/r/#/q/status:open+project:mediawiki/extensions/Score,n,z has the list of patchsets. It's most likely that Szymon Świerkosz (Beau) or GrafZahl would be responding to that critique. But anyone can.... :)

https://gerrit.wikimedia.org/r/#/q/project:mediawiki/extensions/Score,n,z

Looks like everything was merged? Any ETA on deployment? Any place where deployment can be discussed?

(In reply to comment #21)

https://gerrit.wikimedia.org/r/#/q/project:mediawiki/extensions/Score,n,z

Looks like everything was merged? Any ETA on deployment? Any place where
deployment can be discussed?

So it looks like Ifcad0daf was abandoned by Tim on August 24 because Swift was going to be used to resolve the issue instead. Is there a new changeset addressing the issue that Ifcad0daf sought to address?

If so, you may be ready for a final review and deployment. I believe the current schedule is at http://wikitech.wikimedia.org/view/Software_deployments. You need to find a shell "shepherd" (if you will) who will help you get this deployed.

(In reply to comment #23)

It's not ready for deployment.

Just for the reasons MZM mentioned, or are there other code review issues?

sumanah wrote:

I've heard from the code author that he does not currently have time to work on Score. So, if Tim and Arthur could specify what now needs fixing in Score to get it ready for deployment, that would be great; I could ask GrafZahl to fix those, or try to find another volunteer. Thanks.

sumanah wrote:

Tim's fixing up some issues in Score, working on it every Friday, and will be committing into Git when he has more progress.

sumanah wrote:

Specifically, the issue is Swift support -- everything that writes to a filesystem has to be able to work with Swift, and so Tim's working on that.

I noticed that the Swift support has been pushed to Gerrit, so I guess it's waiting for review: Gerrit change 23644

sumanah wrote:

Aaron reviewed the code today and requested a few improvements. Tim or Markus, which of you would like to improve the patchset to accord with Aaron's suggestions?

sumanah wrote:

Markus, do you have time to work on this?

Sumana, I guess so. Will give it a try next week.

sumanah wrote:

Markus, sorry for the nag, but have you been able to make any progress? Thanks.

sumanah wrote:

Actually, https://gerrit.wikimedia.org/r/#/c/23644/ is now merged, so is there anything blocking deployment?

It's tentatively scheduled for deployment on test2 for today: http://wikitech.wikimedia.org/view/Software_deployments#Near-term_to_be_scheduled. So is there anything left to do?

sumanah wrote:

It's deployed to test2 https://test2.wikipedia.org/wiki/Special:Version but unfortunately it depends on OggHandler and needs substantial coding to switch that dependency to TimedMediaHandler. Anyone want to help?

It should be fairly trivial to make the MIDI generation and audio player output configurable and/or dependent on the presence of OggHandler. Would make a good volunteer task :)

(In reply to comment #36)

It should be fairly trivial to make the MIDI generation and audio player output
configurable and/or dependent on the presence of OggHandler. Would make a good
volunteer task :)

Is this a dependency? This bug (bug 33193) doesn't seem to have any other bug dependencies currently. (Surprisingly....) If there are small, discrete tasks that can be worked on, they should be filed as separate bugs and marked with the "easy" keyword.

As it is, I'm not sure what the path forward for this bug is at the moment.

Markus: Would you have some time to address comment 35 by hacking on audio player output etc.?
(Just asking as you're the assignee of this bug report.)

Andre: it seems that bug 43388 has been assigned to jgerber by now.

Tim: So, just to be explicit, we're at a point where this could be deployed to the cluster?

If so, let's do this by the end of the month, probably by scheduling a deployment window where you are available since I believe you would be able to diagnose potential issues. Thanks.

(In reply to comment #40)

Tim: So, just to be explicit, we're at a point where this could be deployed
to the cluster?

No. I just filed bug 46374 ("Score extension broken on test2.wikipedia.org"). And bug 43388 ("Make Extension:Score use TimedMediaHandler for MIDI") is still unresolved.

I'd like to see the Score extension enabled and working on test2.wikipedia.org and test.wikipedia.org for a bit before wider deployment.

Ok, so, Score works on test2, see https://test2.wikipedia.org/wiki/Score

Tim/Markus: anything else in your queue on this bug that needs fixing?

I'm neither Tim nor Markus, but this seems good to go to me. I re-read this bug and it seems all of the known outstanding issues have been resolved.

This needs a deployment window.

Which wikis should we deploy this to? Seems that Wikipedia, Wikisource, and Commons are the obvious targets, though there doesn't seem to be any reason to restrict it from any wiki. Also, all at once, or staged?

(In reply to comment #44)

Which wikis should we deploy this to? Seems that Wikipedia, Wikisource, and
Commons are the obvious targets, though there doesn't seem to be any reason
to
restrict it from any wiki. Also, all at once, or staged?

Just default it on

It's not going to be used instantaneously everywhere due to it being quite a niche feature, and at the same time isn't going to annoy people by making stuff appear that hasn't been requested.

Make a window and turn it on. Any problems with it are only really going to come up with people using it

I guess I might aswell take this bug

Related URL: https://gerrit.wikimedia.org/r/60309 (Gerrit Change I750caf1fe9f0ae2d428f70f11e46fdc4292fd409)