User "Brion VIBBER" posted a comment on MediaWiki.r90747. Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/90747#c18907 Commit summary:
getPath() does the wrong thing under SwiftMedia, but getRel() works for everything. Comment: If I understand right, this is because SwiftFile::getPath() implicitly copies the file to the local filesystem? I honestly find that pretty worrying; getPath() is not expected to have open-ended side effects like that, and this may indicate that the File & LocalFile APIs aren't actually very suitable for accessing remote files... should it be refactored to include explicit concepts of checking out a temporary local file and removing it when done? It looks like the file path will be implicitly deleted when the SwiftLocalFile object is destroyed, which should keep from leaking on batch operations, but that might still mean a lot of unexpected copies being made and deleted. I also see a fully duplicated getHistory method which has had a couple bits swapped out. (It kinda looks like all it needs to do is set $this->oldFileFromRowFactory on the SwiftRepo -- which appears to already be done -- and then just inherit the entire LocalFile::getHistory method?) _______________________________________________ MediaWiki-CodeReview mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
