Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RSS Feed Client #11123

Merged
merged 13 commits into from
Feb 24, 2023
Merged

Add RSS Feed Client #11123

merged 13 commits into from
Feb 24, 2023

Conversation

sawyersteven
Copy link
Contributor

  • PR is based on the DEVELOP branch
  • Don't send big changes all at once. Split up big PRs into multiple smaller PRs that are easier to manage and review
  • Read the contribution guide

Adds a RSS feed client for both nzb and torrents.

Why?

In broad strokes, its similar to a blackhole client in that Medusa will simply grab the result and promptly forget about it, requiring the client to alert Medusa when the download is complete and ready for processing.

The main benefit is that this allows for a download client on a different network to simply read the rss file (which can be exposed easily through any reverse proxy) instead of needing to mount the blackhole directory to download and/or delete torrent and nzb files.

It also allows an effortless one-to-many relationship between Medusa and a download client in case you want to send the feed to several different locations to download simultaneously. I can't think of a particular use case for that, but I'm sure it can help someone.

The frontend changes will need to be built before merging with master. My frontend skills are not great, though I did get this to build and run. Still, it seemed wise to me to let someone familiar with npm and yarn and everything else to do it properly.

@medariox
Copy link
Contributor

Thanks for the PR, it looks great!
Can you fix the test please?


section_data['rss'] = {} needs to be added to the test.

@sawyersteven
Copy link
Contributor Author

Thanks for the PR, it looks great! Can you fix the test please?

section_data['rss'] = {} needs to be added to the test.

Done. You probably saw that I did manage to get the frontend built around my changed (I know almost nothing about npm or any of that workflow) and the massive amount of changes that produced. Is that going to be a problem? I can revert that commit if that would make merging easier.

@medariox
Copy link
Contributor

@sawyersteven
It looks like you built the themes for master, you should build for develop instead. To do that, in the themes-default/slim folder, run yarn install --prod false --force && yarn dev to build the themes for develop.

@medariox
Copy link
Contributor

Your test seems to be failing with Python 3.7, see: https://github.com/pymedusa/Medusa/actions/runs/4217939243/jobs/7352586386#step:5:93

The previous test compared the xml as a string to an expected string. This worked occasionally because the xml writer will sometimes place attributes in a random order.

Also a bunch of formatting changes while I tried to eliminate warnings
@sawyersteven
Copy link
Contributor Author

Seems to be a problem with ElementTree putting xml attributes in a random order, which (of course) worked fine for me when I tested it. I just pushed a better test and the frontend build with the command posted above.

@medariox
Copy link
Contributor

@sawyersteven
Copy link
Contributor Author

Well that's irritating. A classic case of it works on my machine

I'll have to try and abuse it some and see what the problem is.

sawyersteven and others added 4 commits February 23, 2023 23:01
Change import from ET to ElemTree and change whitespace location in expected string to satisfy Flake8. This doesn't affect the test, but unclogs the test output a little.

Recursive test fails on assertion rather than returning bool to bubble up through the nested generators.

Strip newlines from expected string before parsing in ElementTree
@medariox medariox merged commit d43a92d into pymedusa:develop Feb 24, 2023
@medariox
Copy link
Contributor

Will be available with the upcoming release. Thank you!

@sawyersteven
Copy link
Contributor Author

Awesome, thank you!

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.

None yet

2 participants