search

raidenyn / yandexdisk.client

Yandex Disk API Client for .NET
MIT License
40 stars 14 forks source link

FilesClient.UploadFileAsync() does not await upload completion #2

Closed viciousviper closed 8 years ago

viciousviper commented 8 years ago

The UploadFileAsync(...) method may return before the associated upload link has reported completion.

Please provide an extension method similar to CopyAndWaitAsync() - emphasis being on the AndWait.

Further, the existing ...AndWait.. implementation allows only a rather coarse configuration of the delay period between checks for link completion. Could you change the parameter pullPeriod from int to TimeSpan?

viciousviper commented 8 years ago

One more thing: Could you be bothered to upload a new release to NuGet? The fix to this issue combined with your fix for the in-progress JavaScript serialization will be quite an improvement over version 1.0.5 in my view.

raidenyn commented 8 years ago

Hi viciousviper!

I had some problem with MyGet build server, but just now I solved it. So the new nuget package v1.0.7 has been published.

TimeSpan was used because the value less 1 second is bad idea, I guess. It can be problem if you want to use 1.5 seconds (for example), but I don't see any real case for using this value instead 1 or 2 seconds. Do you really see one? Anyway it is not so complicated and I can do it.

Sorry, I don't see the problem with uploading or I don't understand it. Do you mean that after file uploading you don't see it on Disk some time? So Yandex Disk has some undocumented delay for this operation? Or do you mean that UploadFileAsync can return before that http request to API is completed?

Actually, I haven't encountered any errors with this method yet. Can you please show an example code where I can see and test it? Thanks.

viciousviper commented 8 years ago

Hi raidenyn, thanks for updating your package- I'll check it out tomorrow.

I've asked for a shorter delay option on the AndWait operations because I'd expect that moving, copying, or deleting a file should typically take much less than a second, rather like 100ms. This is not substantiated by hard proof, so if you have actually experienced problems with a delay below 1s, then I'll be happy to go with that. On the other hand the workaround I implemented before I found your Extensions class got along quite well with a 100ms delay. Once again I'm asking for this because deleting (or copying or moving) a set of several files or directories would be very slow with a 3s default delay for each individual operation. I see that the Yandex Disk REST API accepts lists of QuickKeys for some operations but it's not always possible to aggregate file system requests into lists before handing them off to the cloud driver.

Concerning the case of UploadFileAsync() possibly returning prematurely - I may have been in error there. I have a unit test suite that repeatedly creates and disposes a test directory for each file operations test in quick succession - and initially I had to fight against frequent errors originating from the Yandex Disk server related to "Resource is locked" or "File is already existent". Turns out I had a runaway Async call somewhere and after fixing that I've since not actually seen any contention of file creation and deletion. Just looking at your code, though, I was wondering if there could be a loophole for a race condition.

I do have some piece of repro code somewhere - I'll recheck and let you know if I find anything.

Cheers, vv

raidenyn commented 8 years ago

HI!

Sorry for my long silence. I have changed *AndWait methods from int to TimeSpan for pull period parameter. I still think that values less 1 second is bad idea, but in some case not integer value can be useful.