Delay subsequent requests to the same host #19801

jNullj · 2023-10-27T10:27:21Z

This adds a delay between two network downloads to avoid hitting rate limits
The delay can be set in Preferences in the RSS tab.
Added both in native GUI and webUI.
I tested this manually on Linux and i did not detect any obvious issues.

Native GUI image of the new setting:

webUI image of the new setting:

Some other things to note which are not directly related:

Please note that i found the webUI to have some bugs not related to this PR, i might PR fixes later, for example changing language, and some settings not getting default value in Preferences and more...
Also while opening this PR i was redirected to CODING_GUIDELINES.md Which encourages me to use uncrustify, After running it i realized many files are not formatted by it, I might get a new PR that formats everything that's missing soon

This commit only adds the RSS fetch delay setting to gui and loads it to to RssSession Part 1 for qbittorrent#8350

part of qbittorrent#8350

Add delay to requests with the same ServiceID The delay length is based on RSS fetch delay Fixes qbittorrent#8350

src/base/rss/rss_session.h

src/gui/optionsdialog.cpp

src/webui/api/appcontroller.cpp

src/base/rss/rss_session.cpp

src/webui/www/private/views/preferences.html

src/base/rss/rss_session.cpp

src/base/net/downloadmanager.cpp

glassez · 2023-10-27T13:16:30Z

while opening this PR i was redirected to CODING_GUIDELINES.md Which encourages me to use uncrustify,

Note that uncrustify does not guarantee full compliance with our coding style. Just read CODING_GUIDELINES.md and follow it in your code contribution.

luzpaz · 2023-11-03T15:07:36Z

ping @jNullj

jNullj · 2023-11-03T20:39:57Z

ping @jNullj

Thank you for the reminder, I may be able to fix the PR this sunday.

ASTX111 · 2023-11-29T12:55:35Z

Any idea when this might be coming? It would be a great update to the appliacation! <3

jNullj · 2023-11-29T19:22:47Z

Due to my new job, my GitHub activity has decreased. Apologies for delays. I'll aim to resume work this weekend. Feel free to co-commit or suggest changes. If maintainers feel the need to edit or create a separate PR due to my pace, I understand. Thanks for your patience.

Co-authored-by: Chocobo1 <[email protected]>

Delay is per ServiceID This removes dependency of RSS_Session from downloadmanager.

Update delay for registered sequential download services. User might change rss fetch delay. This commit updates existing services with new values to take instant effect for the change without the need for a restart. Closes qbittorrent#8350

jNullj · 2023-12-04T21:30:23Z

Updated the PR:
Resolved all suggestions by @Chocobo1 at 8510d96 to e1d5d28
I fixed the issue spotted by @glassez
Added code to update existing delay when the value is changed and merged latest master commits as well.

src/base/net/downloadmanager.cpp

src/base/net/downloadmanager.h

src/base/settingsstorage.h

src/gui/optionsdialog.ui

Co-authored-by: Vladimir Golovnev <[email protected]>

Revert some unneeded changes to optionsdialog.ui intrudoced at commit e79878a This will make diff at merge more readable

Chocobo1

Last comments from me

src/webui/api/appcontroller.cpp

src/gui/optionsdialog.ui

src/base/net/downloadmanager.cpp

Change rss_fetch_delay from int to qlonglong to avoid clipping Co-authored-by: Chocobo1 <[email protected]>

Allow maximum value as user input to remove artificial limit Co-authored-by: Chocobo1 <[email protected]>

glassez · 2024-01-08T06:56:31Z

Unfortunately, I have found that these changes have "opened the door" to undefined behavior and/or race conditions. I want to make some preliminary changes in separate PR, and when it is merged, we will continue with this one.

glassez · 2024-01-11T08:19:42Z

I want to make some preliminary changes in separate PR

#20257

glassez · 2024-01-11T08:25:28Z

@jNullj
It looks like the place where you originally inserted the timer code was the best choice. The illogic that confused me was caused by the inappropriate naming of some things, so I corrected them in #20257. I added the timer code there in advance (with zero timeout), so all you have to do is add the code for setting the configured timeout.

jNullj · 2024-01-12T08:00:54Z

No problem, i will update it after #20257 is merged to avoid reworking this pr if it changes before that.
I noticed you included a copyright notice update, should i add my (and co-authors) name/year in the files once i update them?

glassez · 2024-01-12T08:31:55Z

should i add my (and co-authors) name/year in the files once i update them?

Your - yes.
Are there any co-authors who have made significant contributions to it? Or do you mean those who offered some tips on correcting the coding style, etc.?

jNullj · 2024-01-12T09:41:07Z

should i add my (and co-authors) name/year in the files once i update them?

Your - yes. Are there any co-authors who have made significant contributions to it? Or do you mean those who offered some tips on correcting the coding style, etc.?

Those who offered some tips on correcting the coding style, as in those who reviewed my code and offered changes.
All changes i made which you did not offer, I created myself.

glassez · 2024-01-12T10:27:27Z

Those who offered some tips on correcting the coding style, as in those who reviewed my code and offered changes.

If the proposed changes are trivial, such as correcting the coding style, then we do not take this into account as co-authorship.

glassez · 2024-01-13T05:57:18Z

@jNullj
#20257 has just merged.

…nt to Download Manager

Re-add download delay for same serviceID (same domain and port) after the download manager refactor.

src/base/net/downloadmanager.cpp

glassez · 2024-01-19T17:38:36Z

@jNullj
Thank you!

HanabishiRecca · 2024-01-23T09:47:17Z

@glassez

HanabishiRecca

Sorry, I didn't publish the review.

src/webui/www/private/views/preferences.html

jNullj added 3 commits October 25, 2023 15:06

Add RSS fetch delay to settings

e79878a

This commit only adds the RSS fetch delay setting to gui and loads it to to RssSession Part 1 for qbittorrent#8350

Add RSS fetch delay to webui

5c471f0

part of qbittorrent#8350

Add RSS fetch delay functionality

ae79c73

Add delay to requests with the same ServiceID The delay length is based on RSS fetch delay Fixes qbittorrent#8350

Chocobo1 reviewed Oct 27, 2023

View reviewed changes

glassez changed the title ~~Feat: Requests delay for the same host~~ Delay subsequent requests to the same host Oct 27, 2023

glassez self-assigned this Oct 27, 2023

glassez requested changes Oct 27, 2023

View reviewed changes

src/base/net/downloadmanager.cpp Outdated Show resolved Hide resolved

glassez added RSS RSS-related issues/changes Core labels Oct 27, 2023

jNullj and others added 7 commits December 2, 2023 21:10

Fix code format and style

8510d96

Co-authored-by: Chocobo1 <[email protected]>

Remove redundent code

fad287c

Co-authored-by: Chocobo1 <[email protected]>

Use std::chrono::seconds for fetchDelay

e1d5d28

Co-authored-by: Chocobo1 <[email protected]>

Merge branch 'qbittorrent:master' into feat/8350/RSS-request-delay

dd94380

Add delay to registerSequentialService

7a9dcde

Delay is per ServiceID This removes dependency of RSS_Session from downloadmanager.

Merge branch 'qbittorrent:master' into feat/8350/RSS-request-delay

e72b20c

jNullj requested review from glassez and Chocobo1 December 4, 2023 21:30

glassez reviewed Dec 15, 2023

View reviewed changes

glassez requested a review from a team December 15, 2023 08:38

glassez added this to the 5.0 milestone Dec 15, 2023

jNullj and others added 3 commits December 15, 2023 11:41

Fix code format and style

590c53d

Co-authored-by: Vladimir Golovnev <[email protected]>

Add missing parameter const to functions

a4eb01b

Co-authored-by: Vladimir Golovnev <[email protected]>

Rearnge optionsdialog.ui for better code history

59c3025

Revert some unneeded changes to optionsdialog.ui intrudoced at commit e79878a This will make diff at merge more readable

glassez previously approved these changes Jan 6, 2024

View reviewed changes

glassez requested a review from a team January 6, 2024 14:53

Chocobo1 reviewed Jan 7, 2024

View reviewed changes

src/webui/api/appcontroller.cpp Outdated Show resolved Hide resolved

src/webui/api/appcontroller.cpp Outdated Show resolved Hide resolved

src/gui/optionsdialog.ui Outdated Show resolved Hide resolved

src/base/net/downloadmanager.cpp Outdated Show resolved Hide resolved

jNullj and others added 2 commits January 7, 2024 23:07

Avoid fetch delay clip at webui

aa96086

Change rss_fetch_delay from int to qlonglong to avoid clipping Co-authored-by: Chocobo1 <[email protected]>

Update spinRSSFetchDelay max value

6fa7c4e

Allow maximum value as user input to remove artificial limit Co-authored-by: Chocobo1 <[email protected]>

jNullj dismissed glassez’s stale review via 6fa7c4e January 7, 2024 21:11

jNullj requested a review from Chocobo1 January 7, 2024 21:16

jNullj added 3 commits January 13, 2024 13:13

Merge branch 'master' into feat/8350/RSS-request-delay after improvme…

01509fa

…nt to Download Manager

Add delay to processRequest

64d62df

Re-add download delay for same serviceID (same domain and port) after the download manager refactor.

Add copyright notice

64894d3

jNullj requested a review from glassez January 13, 2024 20:29

glassez reviewed Jan 14, 2024

View reviewed changes

src/base/net/downloadmanager.cpp Outdated Show resolved Hide resolved

Fix copyright notice

8f6218e

jNullj requested a review from glassez January 14, 2024 19:23

glassez approved these changes Jan 15, 2024

View reviewed changes

glassez requested a review from a team January 15, 2024 07:06

glassez merged commit c5d7b62 into qbittorrent:master Jan 19, 2024
13 checks passed

HanabishiRecca reviewed Jan 24, 2024

View reviewed changes

src/webui/www/private/views/preferences.html Show resolved Hide resolved

jNullj mentioned this pull request Jan 24, 2024

Fix typo in preferences.html #20326

Merged

glassez mentioned this pull request Feb 19, 2024

Update Changelog for v5.0.0 #20443

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delay subsequent requests to the same host #19801

Delay subsequent requests to the same host #19801

jNullj commented Oct 27, 2023

glassez commented Oct 27, 2023

luzpaz commented Nov 3, 2023

jNullj commented Nov 3, 2023

ASTX111 commented Nov 29, 2023 •

edited

Loading

jNullj commented Nov 29, 2023

jNullj commented Dec 4, 2023

Chocobo1 left a comment

glassez commented Jan 8, 2024

glassez commented Jan 11, 2024

glassez commented Jan 11, 2024

jNullj commented Jan 12, 2024

glassez commented Jan 12, 2024

jNullj commented Jan 12, 2024

glassez commented Jan 12, 2024

glassez commented Jan 13, 2024

glassez commented Jan 19, 2024

HanabishiRecca commented Jan 23, 2024

HanabishiRecca left a comment •

edited

Loading

Delay subsequent requests to the same host #19801

Delay subsequent requests to the same host #19801

Conversation

jNullj commented Oct 27, 2023

glassez commented Oct 27, 2023

luzpaz commented Nov 3, 2023

jNullj commented Nov 3, 2023

ASTX111 commented Nov 29, 2023 • edited Loading

jNullj commented Nov 29, 2023

jNullj commented Dec 4, 2023

Chocobo1 left a comment

Choose a reason for hiding this comment

glassez commented Jan 8, 2024

glassez commented Jan 11, 2024

glassez commented Jan 11, 2024

jNullj commented Jan 12, 2024

glassez commented Jan 12, 2024

jNullj commented Jan 12, 2024

glassez commented Jan 12, 2024

glassez commented Jan 13, 2024

glassez commented Jan 19, 2024

HanabishiRecca commented Jan 23, 2024

HanabishiRecca left a comment • edited Loading

Choose a reason for hiding this comment

ASTX111 commented Nov 29, 2023 •

edited

Loading

HanabishiRecca left a comment •

edited

Loading