Skip to content
This repository has been archived by the owner on Dec 25, 2023. It is now read-only.

Support upload_processing_limit #246

Merged
merged 4 commits into from
Feb 13, 2021
Merged

Support upload_processing_limit #246

merged 4 commits into from
Feb 13, 2021

Conversation

kamil4
Copy link
Contributor

@kamil4 kamil4 commented Feb 13, 2021

Implements a limit on the number of photos being simultaneously processed during upload. I expected that adding this would complicate the process function but in fact it simplified it (since it was needlessly complicated to begin with).

This implements what was discussed in LycheeOrg/Lychee#903 and LycheeOrg/Lychee#913. The default limit is 4. Setting it to 0 disables the limit.

There will be a trivial corresponding server-side PR that adds a config variable.

@kamil4 kamil4 requested review from ildyria and d7415 February 13, 2021 05:21
Copy link
Member

@ildyria ildyria left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not tested but looks okay.

Tested, BREAKING : you cannot upload more than one file at once.

Fixed.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ildyria ildyria merged commit 26ac13f into master Feb 13, 2021
@ildyria ildyria deleted the processing-limit branch February 13, 2021 12:21
@pchampio
Copy link
Contributor

Also fixes: LycheeOrg/Lychee#443

@@ -50,7 +50,7 @@ let lychee = {

album_subtitle_type: "oldstyle",

upload_processing_limit: 4,
upload_processing_limit: 2,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you change it to 2? And why only in this least important place?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was testing. :'D

@@ -252,7 +252,7 @@ lychee.init = function () {
lychee.admin = !lychee.api_V2;
lychee.nsfw_visible_saved = lychee.nsfw_visible;

lychee.upload_processing_limit = parseInt(data.config.upload_processing_limit);
lychee.upload_processing_limit = parseInt(data.config.upload_processing_limit) || 4;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong. This prevents the value of 0 from being used. I don't think we can use this || syntax for numerals that can be 0.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hummmm indeed, I did not think about this. The problem is that if you send null to parseInt you get NaN which obviously crash all the upload checks afterwards.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants