-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reliable uploads #919
Reliable uploads #919
Conversation
cleaning up FileUploadService.onStartCommand preparing persistant uploads
check FileUploadService.java: //How does this work? Is it thread-safe to set mCurrentUpload here? //What happens if other mCurrentUpload is currently in progress? // //It seems that upload does work, however the upload state is not set //back of the first upload when a second upload starts while first is //in progress (yellow up-arrow does not disappear of first upload)
…ploads Conflicts: src/com/owncloud/android/ui/adapter/FileListListAdapter.java
…ploads Conflicts: src/com/owncloud/android/files/services/FileUploadService.java src/com/owncloud/android/operations/UploadFileOperation.java src/com/owncloud/android/ui/activity/FileDisplayActivity.java
Conflicts: src/com/owncloud/android/ui/activity/UploadFilesActivity.java
added menu for uploadListActivity created separate ConnectivityActionReceiver
add comments filter duplicate photos in InstantUploadBroadcastReceiver
New branches:
Please, if you want to contribute in reliable uploads, start with the User Interface. We prefer to do the rest of the parts because they can include to change some parts in the structure of the project. EDITED to reduce number of branches |
@masensio how did you split the PR? I looked at the UI branch and couldn't find many relevant changes, for example Layouts like https://github.com/owncloud/android/blob/reliable_uploads/res/layout/upload_list_item.xml aren't on this branch so it doesn't make sense to work on this with mostly everything needed missing on the branch :( or at least not to me. UPDATE: My mistake, somehow cloned the master... ups.... |
Added some minor yet untested UI changes to the UI-branch (work in progress) |
Hi @AndyScherzinger, these branches are prepared to do the changes related with each part of reliable uploads feature. Maybe I didn't explain this very well. Sorry. |
@LukeOwncloud , nothing to sorry on your side. We can't expect you are waiting eternally for our response, of course. This is an awesome contribution, and we are really thankful for your work on it. We will continue building from it. Most of the changes to be done are the result of taking so much time to review the PR, while the app kept going forward. This is a very solid base, and deserves to be recognized as such. Thank you very much. |
@AndyScherzinger , sorry, we forgot to push a branch. But you made your view through it, great :) I'll be more assertive. Please, for the moment, focus contributions in the UI stuff. Do not update branch |
I'm not following every comment in this thread - there are a bunch now. But, is there a general feeling from how far we are now from this being generally available? Really looking forward to this update... |
@HeroesDieYoung, we initially targeted to have this ready for release 1.8.0 (in September). But we reviewed the plan this week, and it's too big to complete & test it on time. Probably the visual part of the PR will be ready before the end of the year, and the automatic part a bit later. |
uploadExecutor.submit(uploadTask); | ||
} | ||
StopSelfTask stopSelfTask = new StopSelfTask(intentStartId); | ||
uploadExecutor.submit(stopSelfTask); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @LukeOwncloud pull request is the most valuable since the existence of auto upload in the app!
I found some problems about this pull request. Issue #1129 explain one part of the problem. Files get uploaded multiple times.
Scenario:
- Start an upload
- Start another upload while the other upload is running
this can be done e.g. using auto picture upload and take multiple pictures in sequence.
Expected:
every image is uploaded once
Actual
some images are uploaded multiple times
The Problem is onHandleIntent
will be called multiple times. If calling onHandleIntent
while an upload is running all entries in mPendingUploads
will be added to the uploadExecutor
-queue even if they are already in this queue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @DavidWiesner . We'll watch this for sure.
With the current schedule there is no way we can have this done for https://github.com/owncloud/android/milestones/1.9-current . Moving to next release. cc @rperezb , @MTRichards , @cmonteroluque |
The branch reliable_uploads_action_uploads_view is updated with the last version of master branch. |
@masensio Does this mean that it will now be checked/tested? |
@tobiasKaminsky, at the moment I'm updating with the comments from the reliable_uploads code review. |
Unfortunately, this needs to be delayed one more time. Hopefully, this is the last time. The visual part is almost ready to start QA. The only reason to not wait a bit more to include it in https://github.com/owncloud/android/milestones/1.9.1-current is the need to release the fix on #1442 as soon as possible. Thanks everybody for your patience. Almost there! |
Say hello to #1493, covering this one. Wait your eyes there. |
Even though the branch is being ignored and is thus stale already, for completeness sake: Here a PR for my proposal for reliable uploads: An upload manager which handles both instant and normal uploads and retries failed uploads.
Fixes: #340
Discussion here:
#762
Preview screenshots here:
#765
Upload manager could be used to easily solve a number of issues and to provide requested features, including upload on wifi/charging only, rename on upload, instant upload with delay, extend instant upload to other folders