Skip to content

Direct upload multiple files#8409

Merged
kcondon merged 7 commits intoIQSS:developfrom
MPDL:direct-upload-multiple-files
Feb 9, 2022
Merged

Direct upload multiple files#8409
kcondon merged 7 commits intoIQSS:developfrom
MPDL:direct-upload-multiple-files

Conversation

@haarli
Copy link
Copy Markdown
Contributor

@haarli haarli commented Feb 9, 2022

This pull request addresses and fixes issue #8408.

As stated in the issue, I'm not really sure about the intention of the original code, in particular the splice-method with the 'i+1' deleteCount parameter. So a review would be great.

Generally, this seems to be a concurrency problem, there might still be concurrency issues with this bugfix, but it works much better

Closes #8408

Copy link
Copy Markdown
Member

@qqmyers qqmyers left a comment

Choose a reason for hiding this comment

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

I think this makes sense - fixing an embarrassing bug :-). It's been a while, but I think the code is just scanning the list of pending files and trying to pull one out to work on. It should (with the fix) just pull one entry out of the list (not necessarily the first entry) to process. The original bug would remove more than one entry (more the larger i is - param 2 is number of entries to remove) and only work on the first one. Not a race per se, but a bug that would ignore some files. (I'd have to dig further to see how/when the request to this method wouldn't be trying to get the first element (i=0, which works OK with the bug) but I could imagine it being very intermittent.)

Thanks for the fix!

@kcondon kcondon self-assigned this Feb 9, 2022
@kcondon kcondon merged commit 2d4f56b into IQSS:develop Feb 9, 2022
@haarli
Copy link
Copy Markdown
Contributor Author

haarli commented Feb 10, 2022

Thank you for the prompt reply and merge.
Yes, I thought so, too. Just wasn't sure about the i+1 and if there was an idea behind it. But probably just a confusion between (startIndex, endIndex), like many methods use, and (startIndex, deleteCount), like splice uses. Not embararrassing at all.

Thanks again!

@pdurbin pdurbin added this to the 5.10 milestone Feb 18, 2022
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.

Undesired behaviour when uploading multiple files via GUI and direct upload

5 participants