jenkins-bot has submitted this change and it was merged. Change subject: Separate out `download` and `download_file` commands. ......................................................................
Separate out `download` and `download_file` commands. The Collection RenderAPI expects the result of the `download` command to be a JSON object. We've been overriding this command as the target of the JobStatus url field. Use a new `download_file` command for this purpose, so that we can eventually return a proper "not supported" error to requests for `download`. See comments on https://gerrit.wikimedia.org/r/178454 for some discussion of the error paths involved. Change-Id: I887b3c0051d336602c1bb0fd910c82da459fb085 --- M lib/threads/backend.js M lib/threads/frontend.js 2 files changed, 10 insertions(+), 3 deletions(-) Approvals: Arlolra: Looks good to me, approved jenkins-bot: Verified diff --git a/lib/threads/backend.js b/lib/threads/backend.js index e17a0db..f135929 100644 --- a/lib/threads/backend.js +++ b/lib/threads/backend.js @@ -242,7 +242,7 @@ .spread( function( renderedFile, fileSize ) { // We have something! Create the URL and update everyone! var url = util.format( - "http://%s:%s/?command=download&collection_id=%s&writer=%s";, + "http://%s:%s/?command=download_file&collection_id=%s&writer=%s";, host, config.frontend.port || '80', jobDetails.collectionId, diff --git a/lib/threads/frontend.js b/lib/threads/frontend.js index 5741825..fb209e5 100644 --- a/lib/threads/frontend.js +++ b/lib/threads/frontend.js @@ -297,7 +297,14 @@ case 'render_status': return handleRenderStatus( requestId, args, response ); case 'download': - return handleDownload( requestId, args, response ); + /* Collection extension actually expects a JSON response + * here, giving metadata about the download. In normal + * use this command should be unused, but it is sometimes + * invoked on error paths. But treat this the same as + * the `download_file` command for transition purposes. */ + return handleDownloadFile( requestId, args, response ); + case 'download_file': + return handleDownloadFile( requestId, args, response ); default: throw new FrontendError( util.format( 'Unrecognized command: %s', args.command ), 400 ); } @@ -675,7 +682,7 @@ } ); } -function handleDownload( requestId, args, response ) { +function handleDownloadFile( requestId, args, response ) { var collectionId = args.collection_id; if ( !collectionId ) { -- To view, visit https://gerrit.wikimedia.org/r/178573 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I887b3c0051d336602c1bb0fd910c82da459fb085 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Collection/OfflineContentGenerator Gerrit-Branch: master Gerrit-Owner: Cscott <[email protected]> Gerrit-Reviewer: Arlolra <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
