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

Feature/cap max downloaded #38

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

drmikecrowe
Copy link
Contributor

If you want this, it does the following:

  • Doesn't affect existing operation
  • If the config limit.downloads.podcast_max_new is set to 5 (for example), then 5 episodes are downloaded in the download command

@Keeper-of-the-Keys
Copy link
Contributor

Isn't this something the user decides by themselves, either you download or you don't I'm not aware of auto-downloading?
(I'm pretty sure there would be pitchforks if we had auto-downloading considering the response to episode art)

@drmikecrowe
Copy link
Contributor Author

drmikecrowe commented Oct 24, 2020 via email

@Keeper-of-the-Keys
Copy link
Contributor

Sorry for the extremely delayed response life got a bit in the way.

I'm still not 100% clear on this, since the download command is something the user chooses to run by themselves why do we need to limit this if they download too many episodes they would be barred from downloading more before clearing space.

I'm guessing your use case is different thus necessitating this approach but still would like to understand it more.

(Also one long podcast can be equal in size to lots of other podcasts combined)

@drmikecrowe
Copy link
Contributor Author

I put it on a Raspberry Pi. Stuff You Should Know has 1657 episodes. When I ran download, it almost filled up my SD card.

The download command in gpo downloads everything. No limitations

@Keeper-of-the-Keys
Copy link
Contributor

I see, this is indeed a different use case then what I usually focus on.

Wouldn't using fetch instead of download make more sense for you? Downloads stated purpose is to download all new episodes while you can list & chose using other commands as far as I can tell from the gpo documentation.

@Keeper-of-the-Keys
Copy link
Contributor

To be clear the main reason I'm worried about this change is that it changes the way user-facing a function behaved until now, personally I don't use gpo rather gpodder-core functions as the core lib for a SailfishOS application

@Keeper-of-the-Keys
Copy link
Contributor

Maybe we can change this into:
gpo download 200

Thus the default behavior of download remains unchanged while also allowing the end user to limit how many episodes are downloaded using the download command should they so desire.

@drmikecrowe
Copy link
Contributor Author

So a user with space concerns:

  • Limit their downloads every time they use the download command by adding an optional parameter
    or
  • Set an optional parameter once

It's a don't-care to me, as I'm using my fork. But the need is real, hence my PR. Helms discretion

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.

2 participants