Add plugins for Gazelle-based sites (including RED and NWCD) by pR0Ps · Pull Request #2017 · Flexget/Flexget

pR0Ps · 2017-11-19T20:59:05Z

Motivation for changes:

Add capability to search Gazelle-based sites

Detailed changes:

Use the old what.cd plugin as a base (what.cd was Gazelle-based)
Use a base class to do most of the work
Implement specific sites (NWCD and RED included in this PR) in subclasses
Add 2FA detection (2FA is currently not supported)

To Do:

Implement search interface
Testing
Wiki documentation

liiight · 2017-11-20T07:58:50Z

flexget/plugins/input/gazelle.py

instance variables should be in lowercase

I agree with you, however, these are more like instance constants. They're changed during the initial setup of the class but should stay static after that point.

i dont think it matters. even if they're changed on inheritence, they're still instance variables which is the equivilant of const, and should be lowercase. no biggie though

liiight · 2017-11-20T07:59:10Z

flexget/plugins/input/gazelle.py

should be lowercase

liiight · 2017-11-20T08:00:27Z

flexget/plugins/input/gazelle.py

could replace this to else

Using else would break in the case where no base_url is in the config but one is set up on the subclass so it doesn't need to be configured.

liiight · 2017-11-20T08:00:49Z

flexget/plugins/input/gazelle.py

this check is redundant since you provided a default

liiight · 2017-11-20T08:01:46Z

flexget/plugins/input/gazelle.py

parentheses are redundant here

I'm assuming this is more about the wrapping and less about the method of doing it? I was keeping to 80 chars out of habit but have just noticed that 120 is what's configured in setup.py - I'll unwrap the line since it's only 100 unwrapped.

liiight · 2017-11-20T08:05:46Z

flexget/plugins/input/gazelle.py

redundant parentheses

liiight · 2017-11-20T08:07:21Z

flexget/plugins/input/gazelle.py

should be moved to exception handling

This response is a 200 in a success case. Any other 3xx or 2xx response, while not considered an error to requests, is considered an error at this point in execution.

liiight · 2017-11-20T08:08:06Z

flexget/plugins/input/gazelle.py

avoid redundant line breaks

liiight · 2017-11-20T08:09:19Z

flexget/plugins/input/gazelle.py

this only works for python 3 IMO

liiight · 2017-11-20T08:13:24Z

flexget/plugins/input/gazelle.py

change to be python 2 compatible

cvium · 2017-11-20T09:09:02Z

flexget/plugins/input/gazelle.py

+
+        # Logged in successfully, it's ok if nothing matches now
+        task.no_entries_ok = True
+


One too many linebreaks

cvium · 2017-11-20T09:10:25Z

flexget/plugins/input/gazelle.py

+        self._session.cookies.clear()
+
+        if not force:
+            with FlexgetSession() as session:


We prefer if task.requests is used whenever possible.

cvium · 2017-11-20T09:11:44Z

flexget/plugins/input/gazelle.py

+                    torrent_seeds=tor['seeders'],
+                    torrent_leeches=tor['leechers'],
+                    # Size is returned in bytes, convert to MB for compat with the content_size plugin
+                    content_size=tor['size'] / (1024 ** 2)


We have a util for this

I did see that but didn't think it was appropriate. That function takes a string, guesses the correct units, and converts it to MB. To use it I would have to do something like:

Entry( content_size=parse_filesize(str(tor['size']) + "b") )

Is that preferred over just converting it manually?

It might be a good idea to create a function that does that. It's not the first time we've needed it. Or making it default to bytes if no unit is found.

cvium · 2017-11-20T09:13:01Z

flexget/plugins/input/gazelle.py

+        return list(self.get_entries(self.search_results(params)))
+
+
+class InputRedacted(InputGazelle):


The plugins that inherit from InputGazelle should be in separate files. Not sure where to put InputGazelle... maybe utils?

Yeah, we don't really have a precedence for this. Making it part of core feels weird so I think it's best that all plugins be places together to avoid import issues

There's a lot of coupling between them and I worry that if they were split up it would be really easy to make a change in one of them that would break the others. Keeping them in the same file would help make it easier for people unfamiliar with the code to understand the implications of changing a parent class.

Ex: InputNotWhat inherits from InputRedacted. To set up the params and schema it just uses the parent classes values and adds/changes things. If RED makes a change and adds a releasetype and someone updates the plugin to include it without taking into account the child, it could break the NWCD plugin. Granted this is kinda messy anyway, but alternative was redefining the entire PARAMS structure which seemed like overkill since it's 95% the same.

This also holds true for things like cookies. If the InputRedacted plugin switched from using a cookie called session to one called red_session and the plugin is updated, InputNotWhat will break

To be honest I'm not totally happy with having InputNotWhat inherit from InputRedacted anyway so I'll look into solving this with a better inheritance model.

EDIT: Latest commit alleviates some of these concerns

cvium · 2017-11-22T06:27:16Z

flexget/plugins/input/gazelle.py

A nitpick: We divide imports into four groups:

future

builtin

flexget

3rd party.

So the sqlalchemy import should be moved down a line.

cvium · 2017-11-22T06:29:41Z

flexget/plugins/input/gazelle.py

We don't typically include the TLD in the plugin name unless it's part of the site name... It should just be redacted

pR0Ps · 2017-11-23T01:00:13Z

@liiight @cvium: I think at this point all the comments have been addressed. Is there anything else? If not I'll go ahead with rebasing the commits into something sane.

liiight · 2017-11-23T05:02:34Z

I think it's fine now. Don't worry about the commits, we squash the entire pr on merge

cvium · 2017-11-23T06:38:22Z

flexget/plugins/input/gazelle.py

Is this necessary? We already set a user_agent in task.requests (https://github.com/Flexget/Flexget/blob/develop/flexget/utils/requests.py#L184)

cvium · 2017-11-23T06:39:34Z

flexget/plugins/input/gazelle.py

You don't need to return False. No return statement is the same as return None

The reason I explicitly return False is that having it return True/False vs. True/None is a little more logical IMO. Same reason why I don't return True/0 even though it would behave the same way when used in truthyness comparisons.

cvium · 2017-11-23T06:40:54Z

flexget/plugins/input/gazelle.py

Nitpick: Shouldn't this be called account_info?

cvium · 2017-11-23T06:58:40Z

flexget/plugins/input/gazelle.py

I don't think this linebreak is necessary. We allow 120 chars.

cvium

My recent comments are purely stylistic, so you don't have to do them, but I really want the content_size to use our util (feel free to change it to default to bytes if no unit is provided).

pR0Ps · 2017-11-23T13:25:30Z

@cvium Fixed up style issues and switched to using parse_filesize as per your original comment.

cvium · 2017-11-23T13:42:34Z

flexget/plugins/input/gazelle.py

We already have a plugin for this though https://flexget.com/Plugins/headers

That's useful, I didn't know about that - I've removed the custom UA support from the plugin

Plugin features: - Can function as a search plugin or a task. - Stores sessions in the DB to minimize logins. - Configurable page limit (don't scrape the entire site accidentally) - Extensible classes to allow adding other gazelle-based sites with minimal effort. Provided plugins: - Two generic plugins: - The 'gazelle' plugin should work for the vast majority of Gazelle- based sites, albeit with limited functionality. - The 'gazellemusic' plugin is compatible with the latest version of the public WhatCD Gazelle source. As most Gazelle-based music sites will have used that code as a starting point, this class should work for them. - Two specific plugins: - The 'redacted' plugin is a customized version of the 'gazellemusic' plugin that works with RED's modifications to the Gazelle source. - The 'notwhatcd' plugin is the same thing for NWCD.

liiight · 2017-11-23T15:24:15Z

Thanks. Wanna update the wiki?

pR0Ps · 2017-12-16T04:47:36Z

Added a wiki page: https://www.flexget.com/Searches/gazelle

liiight · 2017-12-16T05:11:08Z

Looks great

pR0Ps force-pushed the feature/gazelle-api branch from 2b94afd to 8179b12 Compare November 19, 2017 20:59

pR0Ps changed the title ~~Add Gazelle input plugin (based on old what.cd input plugin)~~ [WIP] Add Gazelle input plugin (based on old what.cd input plugin) Nov 19, 2017

pR0Ps force-pushed the feature/gazelle-api branch 2 times, most recently from 3d019c2 to 709376d Compare November 20, 2017 07:37

liiight requested changes Nov 20, 2017

View reviewed changes

pR0Ps force-pushed the feature/gazelle-api branch from 709376d to 734cac6 Compare November 20, 2017 08:23

pR0Ps changed the title ~~[WIP] Add Gazelle input plugin (based on old what.cd input plugin)~~ Add Gazelle input plugin (based on old what.cd input plugin) Nov 20, 2017

pR0Ps changed the title ~~Add Gazelle input plugin (based on old what.cd input plugin)~~ [WIP] Add Gazelle input plugin (based on old what.cd input plugin) Nov 20, 2017

pR0Ps force-pushed the feature/gazelle-api branch from 68672c2 to 48a85cf Compare November 20, 2017 09:06

cvium reviewed Nov 20, 2017

View reviewed changes

pR0Ps force-pushed the feature/gazelle-api branch 2 times, most recently from e39c081 to e0c7d8e Compare November 20, 2017 22:32

cvium reviewed Nov 22, 2017

View reviewed changes

pR0Ps force-pushed the feature/gazelle-api branch from 4b8b437 to 3068a70 Compare November 23, 2017 05:43

pR0Ps changed the title ~~[WIP] Add Gazelle input plugin (based on old what.cd input plugin)~~ Add plugins for Gazelle-based sites (including RED and NWCD) Nov 23, 2017

cvium reviewed Nov 23, 2017

View reviewed changes

cvium requested changes Nov 23, 2017

View reviewed changes

pR0Ps force-pushed the feature/gazelle-api branch from 3068a70 to 7935543 Compare November 23, 2017 13:23

cvium approved these changes Nov 23, 2017

View reviewed changes

cvium reviewed Nov 23, 2017

View reviewed changes

pR0Ps force-pushed the feature/gazelle-api branch from 7935543 to a7c9254 Compare November 23, 2017 13:44

liiight approved these changes Nov 23, 2017

View reviewed changes

liiight merged commit 75fd63d into develop Nov 23, 2017

cvium deleted the feature/gazelle-api branch November 28, 2017 07:22


		# Logged in successfully, it's ok if nothing matches now
		task.no_entries_ok = True

		return list(self.get_entries(self.search_results(params)))


		class InputRedacted(InputGazelle):

Conversation

pR0Ps commented Nov 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation for changes:

Detailed changes:

To Do:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pR0Ps Nov 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pR0Ps Nov 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pR0Ps Nov 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cvium Nov 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pR0Ps Nov 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cvium Nov 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pR0Ps commented Nov 23, 2017

Uh oh!

liiight commented Nov 23, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

pR0Ps commented Nov 19, 2017 •

edited

Loading

pR0Ps Nov 20, 2017 •

edited

Loading

pR0Ps Nov 20, 2017 •

edited

Loading

pR0Ps Nov 20, 2017 •

edited

Loading

cvium Nov 20, 2017 •

edited

Loading

pR0Ps Nov 20, 2017 •

edited

Loading

cvium Nov 22, 2017 •

edited

Loading