Giter Club home page Giter Club logo

Comments (10)

samdeane avatar samdeane commented on July 17, 2024

Regarding configuration, could this be provided via an optional delegate method? It could be passed a CK2Configuration object with default values, and asked to return a new one.

If you combine that with specifying the delegate at init time, you've a fairly predictable mechanism.

The delegate can answer any configuration questions when it's called (either once at startup, or whenever the file manager needs them - not sure which would be best).

Users who just want to use the defaults needn't bother to implement the method.

from connectionkit.

mikeabdullah avatar mikeabdullah commented on July 17, 2024

Something like -fileManager:operation:willUseConfiguration:, I guess?

I think this only becomes worth it if different tasks/operations within a session/manager require different configurations. I'm tempted to say people should just have to create a second file manager in which case.

Also, I expect there would be some kind of globally shared config or session/manager that clients could easily use for the default behaviour.

from connectionkit.

MrNoodle avatar MrNoodle commented on July 17, 2024

I may be a little unclear on some of the issues here so please bear with me.

Delegate lifecycle:

In the cases where I use it, my object creates the file manager and sets itself as the delegate. It maintains and releases the file manager when it's done. Having a strong delegate here would create a retain loop and I'm not sure what issue it would be solving besides. Also, I do change delegates in one case. Not sure what issues are involved with this but I'm not seeing any odd effects. Note that things are done serially on my end which may avoid whatever problems you are seeing or anticipating.

Configuration

With timeouts, I've been doing that on the client side entirely. If the file manager doesn't call the completion block in time, I cancel the operation and deal with the timeout. Besides timeout, how many of these configuration parameters are you expecting? Is there a reason why they just can't be properties on the file manager that you set?

Task/Operation API

What do you want them to do beside be cancelable? I'd rather not create a new class if that's the only thing. There's a precedent for opaque handles that can be used to deregister/cancel, such as using blocks with NSNotificationCenter.

Include Task/Operation in delegate methods

Normally, you would use a semaphore for this type of thing. If you look at CK2OpenPanelController, it gets around it by only allowing changes to the operations data structures via the main thread. Personally, I'd rather not get too deep into abstracting out the operation stuff as there are threading mechanisms already in place. If you really want to do it, I'd suggest having separate methods which return the unstarted operation , like -operationToCreateFileAtURL:... NSTimer does something similar where you have a method to start it and one to just return one which you can start manually.

Move a little more in favour of delegate methods over blocks

I'm of the opposite mind on this one. I'd hate to have the delegate have to multiplex the different operations. It would end up being like -observerValueForKey:... which always ends up being a bunch of if/elses. The benefit of blocks is having the call back code in the same context from where it was called where it makes more sense. If you look at the completion blocks that CK2OpenPanelController defines, they are specific to the situation of the caller and it makes sense to have the code in the same area. Defining multiple blocks inline can be awkward but you can always define them elsewhere and store them in a var and I still find it preferable to what amounts to a huge switch statement.

In any case, my 2 cents (or maybe a bit more than that).

from connectionkit.

MrNoodle avatar MrNoodle commented on July 17, 2024

Thinking about it more, if you really want to expand the operation and callback functions, here's another idea on how to implement it:

  • Create a subclass of NSOperation.
  • Have manager methods that return the operation without queuing it.
  • The operation can either:
    • Take a delegate
    • Have blocks set on it.

Note that I'd still want the current methods around for those cases where I want to fire off an operation and just be notified when it's done via a completion block. But for more expanded interactions, I'd prefer this as you aren't forced to have one delegate to handle all operations. You can set the same delegate on all the operations if you want but I think it makes more sense to allow for context specific delegates depending on the operation. You can also have the operations be configurable with timeouts and such. And lastly, you can have the same configurations be settable on the manager and inherited by the operations when they are created (but overridable).

from connectionkit.

mikeabdullah avatar mikeabdullah commented on July 17, 2024

Big thanks for all the feedback, here's some followup:

Delegate lifecycle

Yes, this would create a retain cycle, and deliberately so. The delegate (or some other object) would be responsible for breaking the cycle by closing/invalidating the file manager when it's finished.

I shall have a look through your open panel code to better understand changing the delegate (or does this happen in other, private code of yours?).

Configuration

Interesting to know that you're managing timeouts yourself. I think everything in CK currently operates on a timeout of 60s, but not sure how accurate the libcurl-backend is for that!

Right now I don't have any great idea of what other configuration might be required (FTP encryption settings perhaps?). Could be good to have some caching controls in there one day, too.

Again I'm slightly wary of making this a setting on the file manager, as it raises awkward questions about what happens to existing operations when you make a change.

Task/Operation API

For uploads and downloads, it could be nice to support pausing and resuming them. It also gives scope to have various methods available on these objects to report things like progress on-demand, and perhaps attach your own app-specific data.

Include Task/Operation in delegate methods

I agree a semaphore would solve this. Seems to ask quite a lot of clients though, to get the subtle intricacies right. I like your suggestion of having different flavours of methods, though. Perhaps something like:

- (void)uploadData:(NSData *)data toURL:(NSURL *)url completionHandler…
- (CK2Task *)uploadTaskWithData:(NSData *)data URL:(NSURL *)url completionHandler…

That way, clients using the simpler, immediate API don't have a reference to the operation for race conditions to occur from.

Move a little more in favour of delegate methods over blocks

I absolutely don't want to ditch the use of blocks for indicating completion of an operation/task. That style of API is soooo much more pleasant than without blocks. It's the other callbacks, the one's mid-operation, that I'm considering switching to be delegate methods.

Providing a delegate to the individual operation/task is an interesting approach too; I'll have a think about that one.

from connectionkit.

MrNoodle avatar MrNoodle commented on July 17, 2024

I'm still unclear on the need to retain the delegate in the first place. (and the code where I use it is in my own stuff, not in CK2OpenPanel so I don't have a public example). By doing this, you are forcing the delegate to have a separate step to indicate when things are done. I wouldn't want to impose this without a very good reason.

As for configuration, I'd think that any configuration settings would apply when the operation is created or run and wouldn't affect anything already in progress. Also, per my suggestion of doing an NSOperation subclass, this stuff should be set on the operation itself anyways to allow different timeouts for different operations. I'd only have this stuff in the file manager as a place for each operation to get default values. BTW, in my code, I do a timeout of my own and for certain operations (like upload), I base it on the size of the file. Note that this is a timeout of CK completing the operation as a whole. I'm assuming the timeout you are referring to would be an internal timeout for receiving a response.

Also, if you go the NSOperation subclass route, you already have a completion block. You can add support for other blocks like progress and such. It's also already has support for executing in different ways and canceling so really, it seems like a natural fit.

from connectionkit.

mikeabdullah avatar mikeabdullah commented on July 17, 2024

I think because file management is a complex, asynchronous task, I just like to minimise moving parts where I can. I'm not deadset on changing the delegate arrangement though. I really ought to get an understanding for why you have code that switches around the delegate first at the very least!

I agree that file manager-level settings could well be treated as defaults, with per-op customisation being possible beyond that.

My intent would be to have the same timeout logic as NSURLConnection and friends, which is based pretty much on socket activity, rather than overall transfer times. Anything beyond that I consider to be the client's problem to solve.

One issue with NSOperation's completion block API is that it has no concept of failures etc. That would likely have to be exposed as a property on the operation, which does unfortunately make it very easy to create retain cycles. There's also the argument that the concept of NSOperation as a unit of computation doesn't quite match up to the network-based tasks which CK performs.

from connectionkit.

MrNoodle avatar MrNoodle commented on July 17, 2024

I have individual action objects which do the upload. At the same time, I felt it might be nice to share the file manager so I could reuse the connections. So, each time one of the upload actions has to do its thing, it grabs the shared file manager and sets itself as the delegate.

Right, I figured your timeout was different than mine. And I agree that what I'm doing should stay on my end.

As for NSOperation, ah yes, the error thing would be a bit more awkward in that you'd have to remember to query the operation object for it. Would the error have a reference to the operation? I'm not sure where the cycle would come in here. Also, NSOperation is meant to encapsulate a "task" according to the docs. I believe GCD is smart about not having I/O bound operations clog up the queues (I believe it will just expand the pool of concurrent threads in that case).

from connectionkit.

mikeabdullah avatar mikeabdullah commented on July 17, 2024

Ah, interesting. It's not worth sharing file manager instances like that, as connections are deliberately globally shared. I'll note that in the Readme.

from connectionkit.

mikeabdullah avatar mikeabdullah commented on July 17, 2024

I should probably admit at some point that most of the above list is heavily influenced by NSURLSession's design.

from connectionkit.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.