Skip to content

[add] plex_watchlist: Add a new list plugin to access plex watchlist #3632

Merged
gazpachoking merged 16 commits intoFlexget:developfrom
lostb1t:plex_watchlist
Dec 11, 2022
Merged

[add] plex_watchlist: Add a new list plugin to access plex watchlist #3632
gazpachoking merged 16 commits intoFlexget:developfrom
lostb1t:plex_watchlist

Conversation

@lostb1t
Copy link
Copy Markdown
Contributor

@lostb1t lostb1t commented Dec 8, 2022

see #3488 for info

  • Implement matching against the list
  • Add imdb/tmdb ids
  • Implement editing the list
  • Add guid as identifier and matching

Also added some basic tests

When adding an entry to the watchlist with no guid it will search plex discover and match on supported ids (imdb_id etc) or title.

But maybe this needs to be in a separate lookup plugin: plex_discover_lookup or something.

@lostb1t
Copy link
Copy Markdown
Contributor Author

lostb1t commented Dec 8, 2022

@gazpachoking i think that's everything. Let me know your thoughts.

@gazpachoking
Copy link
Copy Markdown
Member

Looks great to me! One thing I'm not sure is if we should add the new dep directly to our requirements, or make it an optional dep and give user a warning that it needs to be installed if it isn't. Leaning toward the latter.

@lostb1t
Copy link
Copy Markdown
Contributor Author

lostb1t commented Dec 8, 2022

Looks great to me! One thing I'm not sure is if we should add the new dep directly to our requirements, or make it an optional dep and give user a warning that it needs to be installed if it isn't. Leaning toward the latter.

as a user, i would be annoyed if i have to install additional dependencies. It's not a big dependency in size and installation time, so i would chose the more user friendly approach and have it already included

@BrutuZ
Copy link
Copy Markdown
Contributor

BrutuZ commented Dec 8, 2022

Looks great to me! One thing I'm not sure is if we should add the new dep directly to our requirements, or make it an optional dep and give user a warning that it needs to be installed if it isn't. Leaning toward the latter.

I'm for optional dependency too. There are enough of those already to not be a strange procedure to install them.

@lostb1t
Copy link
Copy Markdown
Contributor Author

lostb1t commented Dec 8, 2022

K I'll remove the dependency tomorrow.

@gazpachoking
Copy link
Copy Markdown
Member

as a user, i would be annoyed if i have to install additional dependencies. It's not a big dependency in size and installation time, so i would chose the more user friendly approach and have it already included

I agree it isn't awesome, but we have quite a few plugins that all have deps specific to only that one plugin, and our dep list could get quite large with things that a majority of people wouldn't be using. I've been considering a more formal interface for these sorts of optional dependencies, which would allow installing them with a special command, like flexget deps install and flexget deps update or something, that would look at the plugins you have active in your config, and install/update the needed extra dependencies.

@gazpachoking
Copy link
Copy Markdown
Member

Here's an example of how we do it

try:
import cloudscraper
except ImportError as e:
logger.debug('Error importing cloudscraper: {}', e)
raise plugin.DependencyError(
'cfscraper', 'cloudscraper', 'cloudscraper module required. ImportError: %s' % e
)

Usually in on_task_start, so it fails early

@gazpachoking
Copy link
Copy Markdown
Member

Dep can be added here to make sure the test suite installs it

Flexget/pyproject.toml

Lines 109 to 112 in 724ecfb

[tool.poetry.group.plugin-test.dependencies]
# These are optional dependencies for plugins that have tests in the test suite
boto3 = ">=1.24.89"
subliminal = ">= 2.0rc1"

@lostb1t
Copy link
Copy Markdown
Contributor Author

lostb1t commented Dec 9, 2022

Removed the dependency.

@gazpachoking
Copy link
Copy Markdown
Member

Awesome! Looks good, thank you!

@gazpachoking gazpachoking merged commit 3b1da1d into Flexget:develop Dec 11, 2022
@paranoidi
Copy link
Copy Markdown
Member

Please add documentation here, https://flexget.com/en/Plugins/List/plex_watchlist

I linked it in relevant places

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants