Repository: scrapy/scrapy
Pull request: Closes #6365. Fix overridable methods in MediaPipeline
Closes #6365 

Problem statement:
Fix overridable methods in MediaPipeline
`MediaPipeline` defines several empty or almost empty "overridable" methods, which return things inconsistent with their overrides. I propose making all of them raise `NotImplementedError`. Alternatively `MediaPipeline` should just be made an abstract class and all those methods made abstract methods, but I have no idea if that will break anything (e.g. do all children always override all of those methods?).

Another problem is existing tests, that test specifically that e.g. `MediaPipeline.media_downloaded()` returns a response, which makes no sense to me (normally `media_downloaded()` returns a file info dict), so all those need to be changed or removed.

And another problem, indirectly related to this, is that this interface is very poorly documented, most of these functions are not mentioned in the docs at all, so it's not always clear what should they take and return (and the code uses many of them as callbacks in long callback chains so it's not clear even from the code).

Hints from issue discussion:
A tentative proposal:

* Replace all bodies with `raise NotImplementedError()`.
* Remove all tests that test calling these methods, they are not useful for anything even now.
* Make sure nothing else broke.
* Make the methods abstract, make sure nothing broke.
* Ideally get some 3rd-party pipelines and check that they didn't break either (or just check that they override all of these methods, directly or via FilesPipeline/ImagesPipeline, in which case they can't break?)?

Commit message: Merge branch 'master' of https://github.com/kumar-sanchay/scrapy

Patch to review:

Code review comments:
diff_hunk: @@ -35,8 +31,26 @@ def _mocked_download_func(request, info):
     return response() if callable(response) else response
 
 
+class UserDefinedPipeline(MediaPipeline):
path: tests/test_pipeline_media.py
text: @user1:
I wouldn't reuse the old MediaPipeline implementations here but followed the expected return values. So:

* media_downloaded() should return {} (technically, that's also wrong but better than returning `response`)
* file_path() should return ''

Others I think are fine.

@author:
done