dteviot / WebToEpub

A simple Chrome (and Firefox) Extension that converts Web Novels (and other web pages) into an EPUB.
Other
716 stars 136 forks source link

Feature request: change clampSimultanousFetchSize() #1468

Open gamebeaker opened 1 month ago

gamebeaker commented 1 month ago

Change clampSimultanousFetchSize() so that it is the same as this.minimumThrottle = 3000; I think a variable is more elegant than a function and at the moment the user is unable to overwrite clampSimultanousFetchSize(). Todo:

dteviot commented 1 month ago

@gamebeaker

User is not supposed to be able to override clampSimultanousFetchSize(). It's applied on a per parser (i.e. Site) basis, for sites that don't like rates of more than 1 page at a time.

gamebeaker commented 1 month ago

@dteviot i would argue that this.minimumThrottle is the same as in that if it is defined in a parser it is normally required or you get rate limited. So the user should not change minimumThrottle but are still able todo it if they want. I would be ok with either (the user is able/ unable to overwrite both values). I just want it to be consistent as both option influence fetches/ second.

dteviot commented 1 month ago

@gamebeaker I agree the rate limiting is something a mess. Perhaps we should take a step back, decide what the requirements/goals need to be, and proceed from there.

dteviot commented 1 month ago
  1. User should be able to set rate on a per-site/parser basis.
  2. Default rate is 1 page at a time, 3 seconds per fetch.
  3. Should be as easy for users to use as possible.
  4. Some sites mandate 1 page at a time. (e.g. Royal Road's TOS.) For these sites, users can not override pages at a time.
gamebeaker commented 1 month ago

@dteviot and we don't violate the TOS... image but i get it WebToEpub should not increase the server load substantial like a ddos.

gamebeaker commented 1 month ago
  1. User should be able to set rate on a per-site/parser basis.

Isn't that the case currently? Or do you mean the settings should be saved per site/ parser?

  1. Default rate is 1 page at a time, 3 seconds per fetch.

Change in html default value should do the trick.

  1. Should be as easy for users to use as possible.
  2. Some sites mandate 1 page at a time. (e.g. Royal Road's TOS.) For these sites, users can not override pages at a time.

I would interpret it like this:

class Parser {    
    constructor(imageCollector) {
        this.minimumThrottle = 0;
        this.maxPagesToFetchSimultaneously = 8;

and a function similar to getRateLimit() for getFetchSimultaneously() where this.maxPagesToFetchSimultaneously is upper limit instead of lower limit and that the user is unable to overwrite this.maxPagesToFetchSimultaneously to a higher number. For consistency sake i would delete the option checkbox "Override Minimum Throttle Value".

dteviot commented 1 month ago

Isn't that the case currently? Or do you mean the settings should be saved per site/ parser?

I thought if user makes changes in the UI, they are applied to every site/parser. Or did your changes alter that? And yes, I also mean settings should be set and saved per parser. So, if someone finds site X will allow faster downloads, then they WebToEpub will use that faster speed, without changing for other sites.

gamebeaker commented 1 month ago

Ok i am not sure how this should be implemented.

Or did your changes alter that?

I haven't made changes at the moment just ideas.

I thought if user makes changes in the UI, they are applied to every site/parser.

And yes, I also mean settings should be set and saved per parser. So, if someone finds site X will allow faster downloads, then they WebToEpub will use that faster speed, without changing for other sites.

Aren't these two things contradictory? Example: I am using random sites with random values. a.com is able to handle 8 FetchSimultaneously with 0 sec/ wait. The user changes the options accordingly. -> Now all sites have these new option if they aren't constrained in the parser through clampSimultanousFetchSize() and minimumThrottle (your first scenario). But if the user now changes these options for b.com to 4 FetchSimultaneously and 3 sec/ wait it also changes for a.com to the same options i am not sure how the changes should be saveable per parser but still apply to every parser. (A button that says save download speed for website?)

As an other idea:

class Parser {    
    constructor(imageCollector) {,
        this.minimumThrottle = 0;
        this.defaultThrottle = 3;
        this.maximumPagesToFetchSimultaneously = 8;
        this.defaultPagesToFetchSimultaneously = 1;

In the html droppdown to select add a new option named "default" as the new default value. If this value is selected choose the default value from the parser, which could be overwritten in the specific parser. What is the difference between minimumThrottle and defaultThrottle? (it could be only TOS as the default value should protect before HTTP 429) Same question for maximumPagesToFetchSimultaneously and defaultPagesToFetchSimultaneously. As soon as the user changes the value manually this change is only saved for this one parser. All other parser still have the "default" value. (I don't think this feature is necessary)

I like this option because it provides default values while still allowing the user to change it like he wants. I for example use the library function and if i update the ebooks and there are mostly only one or two new chapters i don't need a slow down for the site. (I know edge case...)

I would argue that these options are under "Advanced Options" and if user break things because they configure it to fast for the website (HTTP 426) they are unlucky the same as when you use your cli with sudo or admin rights and you make a mistake.

dteviot commented 1 month ago

@gamebeaker Obviously I wasn't clear. What I meant is that currently, when you make a change, it's applied to all sites. e.g.

a.com is able to handle 8 FetchSimultaneously with 0 sec/ wait. The user changes the options accordingly. -> Now all sites have these new option if they aren't constrained in the parser through clampSimultanousFetchSize() and minimumThrottle (your first scenario). But if the user now changes these options for b.com to 4 FetchSimultaneously and 3 sec/ wait it also changes for a.com to the same options

What I think should happen (where we want to be) is when the settings are changed, the change only applies to the current site/parser. i.e. The user should not need to manually change the speed settings each time they want to download from a different site.