Skip to content
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

Closed
wants to merge 86 commits into from
Closed

Reliable uploads #919

wants to merge 86 commits into from

Conversation

LukeOwlclaw
Copy link

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

Luke Owncloud added 30 commits October 31, 2014 13:32
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
@masensio
Copy link

New branches:

  • reliable_uploads_action_uploads_view: for changes in UI
    • Icons
    • Changes in layouts
    • Access to upload list from navigation drawer
    • add menu actions to the uploads view
    • add cancel action with the pattern selection
  • reliable_uploads_offline_uploads_queue:
    • manage the uploads queue
    • retry uploads failed by connection problems

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

@AndyScherzinger
Copy link
Member

@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....

@AndyScherzinger
Copy link
Member

Added some minor yet untested UI changes to the UI-branch (work in progress)

@tobiasKaminsky tobiasKaminsky mentioned this pull request Aug 24, 2015
@masensio
Copy link

Hi @AndyScherzinger,

these branches are prepared to do the changes related with each part of reliable uploads feature.
They don't contain any relevant change yet.

Maybe I didn't explain this very well. Sorry.

@davivel
Copy link
Contributor

davivel commented Aug 25, 2015

I am 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.

@davivel
Copy link
Contributor

davivel commented Aug 25, 2015

@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 reliable_uploads_offline_uploads_queue , a hard merge is expected when the stuff in sync_full_folder, that is currently under validation, is merged into master.

@HeroesDieYoung
Copy link

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...

@davivel
Copy link
Contributor

davivel commented Aug 28, 2015

@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.

@davivel davivel modified the milestones: 1.8-current, 1.8.1-next Aug 28, 2015
uploadExecutor.submit(uploadTask);
}
StopSelfTask stopSelfTask = new StopSelfTask(intentStartId);
uploadExecutor.submit(stopSelfTask);

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:

  1. Start an upload
  2. 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.

Copy link
Contributor

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.

@davivel
Copy link
Contributor

davivel commented Nov 12, 2015

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

@davivel davivel modified the milestones: 1.9.1-next, 1.9-current Nov 12, 2015
@masensio
Copy link

The branch reliable_uploads_action_uploads_view is updated with the last version of master branch.

@tobiasKaminsky
Copy link
Contributor

@masensio Does this mean that it will now be checked/tested?
Please let me know if I can help you.

@masensio
Copy link

@tobiasKaminsky, at the moment I'm updating with the comments from the reliable_uploads code review.
When it is ready for testing I will tell you, don't worry 😄

@davivel
Copy link
Contributor

davivel commented Jan 29, 2016

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!

@davivel davivel modified the milestones: 1.9.2-next, 1.9.1-current Jan 29, 2016
@davivel
Copy link
Contributor

davivel commented Mar 3, 2016

Say hello to #1493, covering this one. Wait your eyes there.

@davivel davivel closed this Mar 3, 2016
@davivel davivel deleted the reliable_uploads branch March 3, 2016 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet