-
Notifications
You must be signed in to change notification settings - Fork 114
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
Swift rewrite discussion #65
Comments
@MMasterson Just had a read over this, I'm all for a swift transition how long do you estimate this taking? I'm going to play around and cleanup my branch for delegates and open that to see if I was going down the wrong path. I've been learning objective-c (coming from Android straight into Swift) so I may have made a few mistakes. It sounds like we'll can the objective-c delegates anyway but would be good to test out our assumptions. |
@MMasterson Another idea I had over the weekend, possibly not worth the first pass on the re-write is adding support for iOS 13 Background tasks, now that we'll be using a background session I would hope it will be a simple addition to increase the lifetime of background uploads. I don't have any experience with the API myself. |
Amazing initiative, Mark! I just have two comments
How will this impact current users? Are folks developing in Objective-C able to use the Swift version without problems? I am not sure what the current share of Swift projects is. Also, I am wondering if and how this could impact TransloaditKit, which also uses TusKit internally.
Very good idea! Just want to add that we are currently planing some changes to tus-js-client which will then also trickle down to tus-java-client and tus-android-client. :) |
Just reading over your proposed API covered above, looks like some good improvements there I have some additional code that after successful upload I need to mark some stuff as finished on my side. Thus the This also highlights another question, when we go through the retry flow, instead of stopping when retries are reached. Shouldn't we be calling the error callback to let the caller know it's failed? After having a play I can see that |
Objective-C and Swift are able to be used with each other. The API will change though, so the only impact is users will have to use the new API.
Yes, updates will have to be made to
Can you share those changes? |
Re: @foxware00's comments Looking to have this wrapped by Saturday/Sunday
We will implement something, and provide some documentation for background tasks. Just remember iOS13 is very strict on background processes
I think if you set a retry, and it failes, it should retry X times before firing an error. With this new setup you can have your own retry methodology, for example Do a TUSUpload and don't set a retry amount, then if the upload does fail out, you can fire And yes, a custom session configuration should be able to be passed to |
Thank you. |
Yes @shravanteegala. A simple example of how multiple files could work in this rewrite is as follows var arrayOfFiles = [URL(string: "someFilePath/fileName1.jpg"), URL(string: "someFilePath/fileName2.png")]
arrayOfFiles.forEach { path in
fileName = path.lastPathComponent.deletingPathExtension
var newUpload = TUSUpload(withId: fileName, andFile: path)
TUSClient.shared.createOrResume(withUpload: newUpload)
} and now the delegate methods will return a func TUSProgress(upload,_ uploaded, remaining) {
if (upload.id == "fileName1") {
//fileName1 was referenced - do something for that upload
fileName1ProgressLabel.text = String(format: "%@ out of %@", uploaded, remaining)
}
if (upload.id == "fileName2") {
//fileName2 was referenced - do something for that upload
fileName2ProgressLabel.text = String(format: "%@ out of %@", uploaded, remaining)
}
} |
@MMasterson . |
@shravanteegala yes of course, the uploads will happen async/off the main thread to avoid blocking the UI. However that is not something you'll have to worry about when implementing the library, it'll be taken care of behind the scenes. |
@MMasterson do you have your latest code, would love to have a crack at it? |
@MMasterson One more thing I want to share is that |
@foxware00 you can check the @StackHelp I'm sorry, I don't fully understand your issues. Can you provide another example of your problem? |
Looking to wrap this up tomorrow & Sunday and hit all the points listed in the og post |
@MMasterson thank you. I had my own punt at migrating it to swift earlier in the week. Managed to get it all working in a similar vain to the current lib. Trying to as closely follow your proposed API. I did notice however the task of background processing is an interesting one. We'd need to chunk the file into smaller files and uploads those and the dataTask API doesn't properly run in the background. Using uploadTask with an actual file enables much better background running. Im not sure I'll have time to look into it this weekend, but I didn't know if you'd tried tackling this problem previously? |
@MMasterson Yes, Let's say when I create an upload, I have this path |
When will I get the new Swift code? @MMasterson |
hi, |
@chandu4ugandhi @shravanteegala @StackHelp @foxware00 @StackHelp @foxware00 |
Cool.. Appreciate your passion and commitment.
|
Committed a bunch of stuff today relating to the chunking & networking - getting it more ironed out. Feel free to take a look in the swift dev branch. Will do the last bits and test over the next day or so |
I have checked and most of the stuff completed in it but delegates and some other stuff remaining |
Please check the readme of |
@MMasterson I am trying to upload with
And I get this error You can see it takes fileName 2 times here |
@MMasterson when will you resolve this issue? |
@MMasterson I got the same error with this update as well.
|
from // From TLPhotosPickerViewControllerDelegate
func shouldDismissPhotoPicker(withTLPHAssets: [TLPHAsset]) -> Bool {
// I set a global array to the array supplied here, for access later, but not super needed
self.selectedAssets = withTLPHAssets
/*
temp copy media file is the only way I see to get the file path.
I only use the first element of the array, but you can cycle though and do the same process to add multiple files
*/
//Run this for each element, I only use the first
var file = selectedAssets[0].tempCopyMediaFile { (url, string) in
//Create an upload object
var upload = TUSUpload(withId: "File", andFilePathURL: url, andFileType: ".jpeg")
// Pass it to TUSClient, this can be done multiple times, it'll add it to TUSKit's `currentUploads`
TUSClient.shared.createOrResume(forUpload: upload)
}
return true
} |
Its Working fine for images.😀 But when i choose video's 🎥 PHImageManager.default().requestAVAsset(forVideo: mPhasset, options: options, resultHandler: { (asset, audioMix, info) in |
@shravanteegala thank you for pointing that out! Working on a patch for other file types, will report back what the issue was and why it was triggered. |
hey @shravanteegala A new version was just uploaded - can you update. I don't think it'll fix your issue just yet, but just to stay up to date. What is at line 2460 on ChattingViewController? |
@chandu4ugandhi @StackHelp please update try using if you get an error, please try wrapping the file url with If error persists, please let me know. You can still upload via data for now
You can grab the fileData with |
5 Days ago new updated version is implemented for multiple files uploading based on id and //When you have a file, create an upload, and give it a Id. But my problem is after uploading more than one images then immediately i put app it background at that time app is crashed. Can you give me updated version number . for i'll updated to my project. |
@MMasterson Just wanted to ask if there are any updates on the Swift version? :) |
Yes sorry, will commit some updates this week. My main gig has been pretty busy. PRs are always welcome!! |
@MMasterson how do you specify custom headers and metadata for the upload? I found this in the docs for the Objective-C version, but not seeing anything in the new Swift docs
|
After the first file upload, I could not upload again and I found the reason for this. |
@jsahoo There isn't one currently, but can add to the next release. @YazilimTuneli will check into |
@jsahoo custom headers are in the |
Thanks! |
I'm seeing that all the signatures for |
@sripberger there are plans to bring back support for an already created file/id. I have some early support code for it I need to clean up and test a bit. Are you trying to upload to Vimeo or your own TUS instance? |
We are trying to upload to our own tus instance, which happens to be What I'm getting at is not re-using an already-created file id, but the Create extension, where you make a POST to the root of the TUS endpoint, and it gives you back a Location header which contains an ID generated by the server. |
So, after talking with my co-worker it looks like we're talking about two different things. The ids provided to Is there any way to get that server-side id after an upload completes? |
@sripberger yeah we can definitely fix that up. I'm curious as to what ID TUSD uses instead of the one supplied by the client. I see that we're sending the ID to TUS is in base64, could that be the culprit of id's not matching? cc'ing @Acconut here as well |
The id the server uses is generated with this: https://github.com/tus/tusd/blob/master/internal/uid/uid.go I'll try to clear up some confusion, though. The situation is that we have the server set up to create a database entry for a completed upload. The client then needs the id of this database entry in order to proceed with further operations on the file-- associating it with other database entities, like posting a video to a board of videos or something. For simplicity, we are simply re-using the server-side id created by tusd, which works fine since it is both cryptographically secure and close enough to a v4 uuid. So we just need the client to be able to get that id so it can do stuff with files after uploads have been completed. Our iOS engineer was eventually able to get it from the When I asked this question, though, we were definitely mixing up the "id" property on TUSUpload instances and the one used on the server side. That was the cause of the mismatch, so this is largely just a misunderstanding. Thanks for your help, though. :) |
Exactly, that's all correct from tusd's point of view :) I cannot comment on TUSKit's part though. |
I also noticed this, and for now have to manually set status to .ready for each upload |
@andreaslindahl I believe I had patched this in an updated version of the library, but maybe I am mistaken. Can you check what version of the library you're using? |
What if we have dynamic upload URLs? I see upload URL is set in config in AppDelegate, but what if actually I want to do this inside Controller after I get response from API which contains my upload URL. |
You can create the config anywhere and don't have to do it in your AppDelegate |
I'm using version 2.1.5.alpha |
Hi @andreaslindahl, I understand that but...
This gets called on the initial ViewController, and inside TUSClient:
Here we error out because config is not yet set. |
@deeplinkage @andreaslindahl, I recommend putting the config in the app delegate. You'll need to set a URL there, however, you can change the upload URL at anytime using For the next release we could add a better solution, such as adding property to the TUSUpload class that overrides the |
Re writing the library in swift in the
swift-development
branchoriginal gist
Proposed changes for TUSKit
Goals
Proposed new usage
Remaining tasks
Custom Session Configs to be passed to clientsTUS Create Post CallTUS Upload Via ChunkingState persistenceSuccess file creationSuccess file uploadFailed file storageThe text was updated successfully, but these errors were encountered: