petkaantonov / bluebird

:bird: :zap: Bluebird is a full featured promise library with unmatched performance.
http://bluebirdjs.com
MIT License
20.44k stars 2.33k forks source link

A legitimate use for progress? #532

Closed shaunc closed 9 years ago

shaunc commented 9 years ago

I realize that the progres api is deprecated. I have a use for it, which I can't think of another way to implement -- easily -- alternate suggestions are very welcome!

I am modifying an ORM to support transactions. I want to be able to "close" a promise chain in a particular transaction without having to have to thread a transaction object through all the calls. Instead of passing the transaction in, my alternate solution is to pass an object out which will defer execution until given a transaction.

As the API and all the code is already passing bluebird promises out, a seemingly simple solution presents itself: at the bottom of the chain, when a query is ready for a transaction, it passes back a promise in which it calls "progress" with a callback. The transaction wrapper registers a progressed() handler which receives the callback, and calls it with the transaction, which then allows the query to proceed. (The transaction wrapper can check to make sure that it doesn't react to other progress() calls.)

In another thread about promises, I read one of the maintainers claim (in other words) that "instead of progress you should use events". However, here I need to know which promise chain I am in (as there could be other transactions also in progress). I don't see how I can do that with events.

A dummy version passes some simple tests... I'm ready for to start more extensive work. Is there anything wrong with my approach? Is there an alternate solution? Is there, for instance, some way to identify which promise chain I'm in up to some "checkpoint" (ie the transaction wrapper)?

If this is a legitimate use for the progress interface, I would be very pleased if it were un-deprecated, with appropriate warnings put in for other uses for which it may be less suited.

benjamingr commented 9 years ago

You can use the disposer pattern like other ORMs do - that is, have a function take a function that returns a promise - and only executes that promise when that transaction is successful. I'm really not sure what progression even has to do with your use case.

shaunc commented 9 years ago

The disposer pattern is good for making sure the transaction is committed/released, and I should use it, but it doesn't speak to my problem: Suppose I have a complex tree of calls:

openTransaction->a()->b()->c()-> .... z()

(with branches cut off for clarity :)).

z() needs the transaction (their might be several calls to z() in leaves). I want to use the fact that the whole tree passes back a promise so that I don't have to thread the transaction through all the intermediate objects. Instead, z() passes back a promise from a deferred, and also calls

deferred.progress(function (transaction) { 
    do_query(transaction).then( function(result) { deferred.resolve(result}; )} );

Then in "openTransaction I have:

... open transaction ... .then( function(transaction) {
    promise = a(...);
    promise.progressed(function (callback) { callback(transaction); });
    ...
   return promise;
   }).then( function(res) { ... close transaction and pass back result ... } );
shaunc commented 9 years ago

As an alternate to this with events, I could imagine that z() would subscribe to some event "transactionReady". Then openTransaction() would emit the event "transaction for promise chain "FOOBAR" is "transaction". z()'s listener would check its promise to see if it was in the chain of FOOBAR, and if so, execute and resolve.

However, I don't know how or if its possible for z to check if its promise is in the chain of some promise (returned by a()) that openTransaction receives.

cmwelsh commented 9 years ago

Seems a lot more complicated and magical than just passing the transactions around...

Bookshelf uses Bluebird and supports transactions:

http://bookshelfjs.org/#Bookshelf-transaction

shaunc commented 9 years ago

... in other words, I'm using "progress" to send an event that bubbles up the chain of promises like events bubble up the DOM tree. Of course there are pitfalls to using such events -- but they have perfectly well-defined semantics. It would be great if bluebird supported this sort of usage. (e.g. some sort of "stop propagation" call. Also some way to check if I'm at the root of the chain or not...).

shaunc commented 9 years ago

Vis those interfaces: "stopPropagation" could be achieved by "progressed" returning false. What I mean by the other suggestion is to have a global "progressed" registration that catches all progression calls whose propagation isn't stopped.

shaunc commented 9 years ago

@cmwelsh -- From the client point of view, its certainly a lot less complex. Passing transaction through hooks & so forth is a pain, and doesn't contribute anything to understanding code that shouldn't have to care whether its wrapped in a transaction or not.

As far as I can see unless you can see an issue with my analogy to events in the DOM tree it shouldn't be that magical. UIs use events like this all the time. Yes they can be abused... but in this case the semantics are pretty clear.

shaunc commented 9 years ago

@benjamingr (@gaearon) -- It seems that the discussion around deprecation of progress() (e.g. in https://github.com/petkaantonov/bluebird/issues/91) brings up:

1) How does progress from separate branches get combined? 2) Performance (because of lots of branches). 3) Conflicting protocols.

I would answer:

vs 1) via analogy to DOM events, you don't combine -- the events bubbling from separate branches are separate events.

vs 2) This could be the case, but in my case the leaves -- the calls to z() which execute queries are expensive, so messages passed through the promise chain are not going to be the bottleneck. Indeed, these messages could also be used for optimization: If there were 1000 leaves z() could send "I'm ready to execute query "..." and I don't care about the return value, only whether it succeeds. Then the transaction could collect all the queries and send as a single script.

I'm sure there are "bad" uses as well where progress is the bottleneck. The best strategy for the library, I would think, would be to document this.

vs 3) A perennial and typical problem in javascript. In my case I could check the identity of the prototype of the callback and ignore the progress report if it didn't match. I don't see why a similar strategy couldn't be used by any other handler.

Going with the analogy to scoped events in DOM trees imply some other restrictions -- e.g. progressed() handlers closer to the source are called before ones further down the chain (and ideally have a way to stop propagation). I could live without this but it would be nice if actual behavior were documented.

benjamingr commented 9 years ago

If you want context across the transaction just use .bind?

On Mar 19, 2015, at 04:14, Shaun Cutts notifications@github.com wrote:

@benjamingr (@gaearon) -- It seems that the discussion around deprecation of progress() (e.g. in #91) brings up:

1) How does progress from separate branches get combined? 2) Performance (because of lots of branches). 3) Conflicting protocols.

I would answer:

vs 1) via analogy to DOM events, you don't combine -- the events bubbling from separate branches are separate events.

vs 2) This could be the case, but in my case the leaves -- the calls to z() which execute queries are expensive, so messages passed through the promise chain are not going to be the bottleneck. Indeed, these messages could also be used for optimization: If there were 1000 leaves z() could send "I'm ready to execute query "..." and I don't care about the return value, only whether it succeeds. Then the transaction could collect all the queries and send as a single script.

I'm sure there are "bad" uses as well where progress is the bottleneck. The best strategy for the library, I would think, would be to document this.

vs 3) A perennial and typical problem in javascript. In my case I could make check the identity of the prototype of the callback and ignore the progress report if it didn't match. I don't see why a similar strategy couldn't be used by any other handler.

Going with the analogy to scoped events in DOM trees imply some other restrictions -- e.g. progressed() handlers closer to the source are called before ones further down the chain (and ideally have a way to stop propagation). I could live without this but it would be nice if actual behavior were documented.

— Reply to this email directly or view it on GitHub.

spion commented 9 years ago

@shaunc I tried solving this and came up with this class (examples here) - its a lot like a promise except nothing really gets executed until the last step where the final instance takes a transaction and propagates it to everyone else.

Its relatively flexible, e.g. you can return a promise in the middle of a chain (or just a value)

note: I should rename it as at this point its far from a monad (breaks a lot of laws :)

shaunc commented 9 years ago

@benjamingr -- are you suggesting something such as z() binding an object with "getTransaction" to its promise? But then what happens when the chain encounters other promises joining in (all(), etc...)? Or what if a user rebinds the promise?

@spion -- interesting -- very nice! In fact I first thought of inheriting from Promise -- but it isn't very inheritance friendly (e.g. look at the constructor). One drawback I see is that, with this approach, if you want your users to be able to treat what you return as "just a promise" you essentially have to recreate the whole promise interface (all, reduce, 'race'...), whereas (modulo progress being deprecated :)) my proposal can leverage the power and well tested/maintained interfaces of bluebird.

shaunc commented 9 years ago

@spion -- hmm... I guess its probably possible, but how would you implement savepoints in your approach? Currently, if I'm not told I'm the top of the chain, I start a transaction only if there are no other transactions running. If there is one I wait w/ progress() call in case I'm nested, and start first waiter when last started transaction is finished. That way, the library and users writing libraries can wrap sections they want to be atomic oblivious to enclosing transaction (or its lack).

The drawback of my approach in this case is that a bunch of transactions started with Promise.map() would get unnecessarily serialized (if there is in fact no enclosing transaction -- otherwise they get necessarily serialized :)) -- that is why I would like "Promise.unhandledProgress( ... )" which would allow me to avoid this heuristic.

shaunc commented 9 years ago

@petkaantonov -- I surmise that you were one of the movers and shakers behind getting the progress() api deprecated. I discuss some of the specific drawbacks above; I guess the overall reasoning behind deprecating progress was that it was a solution looking for a problem (which wasn't better solved elsewise).

IMO I've found a problem -- phased execution based on events scoped to the promise chain -- for which the progress API is the best solution currently available (though it could be improved with a few niceties that suggest themselves given this purpose). I'm quite interested whether you find my argument persuasive, or whether you would suggest a different alternative. Bluebird is a great library. Though I could roll my own wrappers like @spion, I'd be very pleased if I could leverage bluebird directly in this apparently unexpected fashion.

spion commented 9 years ago

@shaunc there is no savepoint support, so no options to wrap only parts of the code as atomic.

With a careful rewrite that always calls .then to create new promises, and that renames the chain method to then, it may be possible to write a savepoint queryable instead of a transaction queryable - and when that is given to the monad via execWithin(savepoint) it will return a new TxMonad (containing 2 additional queries around it to begin and end a savepoint) instead of a promise. Then when a bunch (or a chain) of those are given execWithin(tx) they will finally return a promise.

However because of recursive thenable assimilation in Promises/A+ and ES6 promises, its not recommended (and probably bugprone as well) to add the then method to anything that is not a promise. Which means that the rewrite will have to handle both cases (regular promise and savepoint TxMonad) separately.

In a language with real support for monads (like PureScript) this would be much easier...

shaunc commented 9 years ago

@spion -- why can't TxMonad be a promise, whether it wraps a transaction or a savepoint? It does impose some additional constraints, and would have some private interfaces, but does it do anything that would necessarily break the spec? Serializing promises in map() for instance, doesn't break the spec AFAIK. (And if it doesn't break the spec, why not use a bluebird Promise instead? :))

spion commented 9 years ago

Promises are eager while TxMonad is lazy. If a TxMonad is assimilated by a normal promise or consumed by a function that creates a new promise out of it, (e.g. Promise.all) the resulting standard promise will become unresolvable as there will be no runWithin method to provide the transaction anymore

shaunc commented 9 years ago

Ah right -- but this problem doesn't apply to use of progress to pass the transaction, right? At least for case such as with all() etc. The one case that I can think of where using progress breaks is when a user explicitly breaks the chain as in:

function (thenable) {
    return new Promise( function(resolve, reject) {
        thenable.then(resolve).catch(reject); })
}

This would have to be rewritten as:

function (thenable) {
    var d = Promise.defer();
    thenable.progressed(d.progress).then(d.resolve).catch(d.reject);
    return d.promise;
}

I was thinking of creating a helper for this situation.

spion commented 9 years ago

Well, using progress for anything other than to signal the progression of the operation seems very wrong to me. YMMV

benjamingr commented 9 years ago

@shaunc I feel like we're being unfair to you here because we're all really busy now.

Progression was deprecated for many reasons - the major one is that it was not a sound abstraction and aggregation never really worked. @kriskowal and @domenic wrote about it at length. It's not only deprecated from Bluebird, but also from Q and other libraries and will never enter native promises.

I promise I'll be available to discuss this and consider this in depth later (in a few weeks) but at the moment I simply lack the time. Meanwhile - I suggest you look into what Kris and Domenic wrote and describe your use case in a clear and simple document (preferably with code).

Sorry, and thanks.

shaunc commented 9 years ago

@benjamingr -- your gracious reply is the sign of a well managed project. I am only "jumping up and down" about this because I'm also busy and may be making a risky decision on my own project. On the one hand, I realize I am climbing out on a limb you have already warned will be cut off, but on the other hand this approach allows me to avoid factoring through transactions in a mass of legacy code. I'll look for their articles. I've already written a fair amount, but will see if there is anything I should add (or perhaps remove :)).

@spion -- I am signalling progression: the leaves signal "I've progressed to the point that I need a transaction -- could you tell me what it is?" The interface is clean, insofar as it can avoid other progression noise (though perhaps this could be a performance problem if there were really lots of messages to ignore). Is it the two-way handshake that bothers you? It decouples all the intermediate code from having to deal with the transaction, or to have to chain a separate non-promise object.

You've done some great work... I'm tempted to be swayed by your opinion... but its a little too unspecific for me to get my head around. Could you elaborate?

spion commented 9 years ago

@shaunc AFAIK the intent of progression has always been to report a number estimate which would generate a progress bar or estimated time to completion. Infact, one idea that @kriskowal had to get progression to compose was to only report estimated time to completion: that way, Promise.all([p1, p2]) has a progression value Math.max(p1.ETC, p2.ETC), Promise.race([p1, p2]) has Math.min(p1.ETC, p2.ETC) (of course once you get to chains that gets tricky)

Where I worked at the end we decided not to pursue the TxMonad approach, instead we created a Transactionable base class that all other classes inherited from, and its constructor took an options object which contained the transaction (or another Transactionable). This in turn enabled extra context to be aded in some situations (e.g. there is a UserTransactionable class that takes both a transaction and the user that is performing the action).

This seemed to work well for us, because we use a service approach rather than the more traditional model approach (to avoid the fat models with billions of responsibilities issue) but I'm not sure how it will work with an ORM. I do think that ultimately something like TxMonad is the best way to go for an ORM...

As for elaborating on TxMonad, what would you like to know?

Regarding it being hard to implement in JS: Unlike in an OO language, in a FP language like Haskell or PureScript you would only implement two functions for every monad - those are bind (a.k.a. flatMap) which corresponds to promise.then and return (a.k.a. unit) which corresponds to Promise.resolve. Everything else is implemented as functions that work on any monad based only on those two "methods", so things like mapM(http://hackage.haskell.org/package/base-4.7.0.2/docs/Prelude.html#v:mapM), liftM2 (like Promise.join), filterM etc are reusable. The key thing here is unit (Promise.resolve) though - Haskell resolves the appropriate monad type and implementation based on the return type, which unfortunately can't be done in JS.

I could however try and refine TxMonad to support nesting (and savepoints). Its a pretty isolated module so I could also extract it outside of anydb-sql and put it on npm. There are two approaches and I could try to construct some examples if you like...

shaunc commented 9 years ago

@spion -- Thanks! [Sorry, mispelt you as the spy library. :)] I see progression as a way for root and leaf of nodes of a call tree to communicate without forcing intermediate nodes in tree (perhaps written by someone else) to do anything except properly chain promises. Sending numerical progress is a simple application. Labelling progress reports by branch seems to be a bad idea -- just as root and leaf don't want intermediate to have to care about their communication, they also shouldn't want to care about intermediate details of call tree (except for specialized applications like error reporting or profiling). Thus -- if a leaf wants to send something without a key, it should make sure it commutes -- e.g. "tick", "tick", "tick" ... :)

I'm still investigating my proposal to make sure that everything works as I expect. If it does I'll follow up on @benjamingr's request for a single document & code.

shaunc commented 9 years ago

@benjamingr -- I am trying to write a document up. Do you have any links to discussions by @kriskowal and @domenic. I have found references in issues concerning compositionality, e,g,

https://github.com/kriskowal/q/issues/463

Also https://lists.w3.org/Archives/Public/www-dom/2013JulSep/0116.html mentions incompatibility with error handling and also with extensions to co-routines. However, I didn't come up with the original discussions.

IMO (only partially informed so far) users had unrealistic expectations of the interface, and pushed it from being just notifications (like scoped events) to things which would interact with the promise chain as they bubbled. I'm specifically interested in decoupling from the intermediate chain, so all of that seems like (indeed) a bad idea, but also completely unnecessary. I don't understand what the issues are that domenic mentions, though.

benjamingr commented 9 years ago

@shaunc any update on that document? I have time to review it now if you're done with it

shaunc commented 9 years ago

@benjamingr -- You work on weekends too? :) I have a transaction wrapper I'm testing now and was planning on putting it up together with the document on Sun eve. ... does that miss your window? I'll try to get things together tonight if so.

shaunc commented 9 years ago

@benjamingr -- here you go: :)

https://github.com/schematist/xwrap/blob/master/transactions-and-promises.md

Now part of "xwrap" transaction manager. Sorry for the delay. I wrote this first as part of my ORM, but then realized my ORM can be completely oblivious to transactions, and even so a client can use xwrap to create transactions.

@spion -- happy to have your comments as well! @cmwelsh -- in fact bookshelf also could be written so as to be oblivious to transactions. :)

benjamingr commented 9 years ago

Replying to your gist inline here:

There are a number of different tools that can interact with a database: a basic driver, ORMs, reporting & analytics tools, maintenance tools, REST interfaces, perhaps connectors to other databases, etc. Ideally, all of these modules should be composable.

I don't understand why an ORM should be composable with a REST interface. They're just two different modules in the systems - they should be used in conjunction but I'm not sure composition is the right word.

But what if you need to use transactions? A REST package may want to wrap interactions in transactions, but a reporting package many not use transactions, and a maintenance package may use its own transactions.

Typically transactions are done with the disposer pattern (pretty much as spion as shown) - I've suggested this syntax to two of the biggest node ORMs and both have adopted it since.

    transaction(function(t){
         return somePromiseWith(t); // transaction commits or rolls back based on transaction
         // use promise chaining here to create query
    }).catch(function(e){
        // you get here if rolled back
    }).then(function(results){ 
        // results available here, transaction committed
    });

In some environments, when tools share a common database driver or ORM, composing packages that use transactions is fairly straightforward. For instance, in python, using django, a view that generates a web page can be wrapped in a decorator which causes everything that the view does to be wrapped in a transaction:

That's very nice, I know for a fact Petka has some pretty interesting ideas about thread-local storage. Typically in node domains are used for thread-local-storage but they're deprecated and suffer from a lot of issues. You can pass context in a promise chain with bluebird using .bind which sets the this value. It's pretty powerful. In addition you can also expose a closure (or block with let) variable and hold thread-local things there. .

Any utility called by view_foo -- even a plugin written by a third party which has no notion of transactions -- can get a database cursor:

This sounds a lot like the disposer pattern shown above - the only caveat is nesting one level.

Unfortunately, in node's asynchronous environment, it would seem this isn't possible. As (the equivalent of) view_foo processes, other views might start processing while view_foo waits on i/o. There may well be several different database clients in use -- but only one with the transaction initiated by view_foo. How could a utility plugin know which client to use, unless it was written in advance to accept a transaction?

I can see several solutions to this - one would be to take a "db inside transaction" object instead of a plain DB object when it takes the database parameter - this would cause all the data to run in the transaction context.

The python runtime will switch contexts while waiting for i/o in a fashion similiar to node, and yet it is still possible to get code that is oblivious to the transaction state to work! This is possible because the "greenlets" used by gevent have identity accross context switches.

This is a very nice property - it's similar to domains in node (which had a right idea done poorly) or fibers which some people also like. It does feel very magical sometime and the more I write code in C# (which has this property) the more I like having explicit context (on the request and response objects) which always does what I expect.

In node this technique seems impossible to utilize: each asynchronous event starts an anonymous call-stack. Though the C++ code might know how to identify these events, there is nothing for a javascript program to refer to, unless it passes information from one call to another, including threading it through 3rd party libraries.

Again, domains/fibers/.bind/explicit context - choose one. I like explicit context myself but to each their own.

The Promise specification is a brilliant framework

Nitpick, interoperability specification for libraries - not a framework

explaining promises

The mechanism by which we "refer" to the chain is to pass a message up the chain using the "progress" interface, and wait for an answer. At the root, where the transaction was started, a listener for this message is registered. Whenever a message in the correct form is received, it passes back the client which executes.

Of course, the chain of calls is really a tree, and only exists ephemerally. But if leaves explicitly wait, then the chain between root and leaf will exist, and the client can pass.

Actually, it's not really always a tree - I've seen diamonds and graphs plenty of times in real applications - but I get the point.

Here, the leaf node creates a promise but doesn't resolve it. Instead the resolution callback is stored in the request object and sent up the chain (as "self") to the wrapping transaction.

Right, this is basically like a handicapped RxJS Observable.

The transaction calls the class method handle, with the transaction and its own promise, which will be attached to the leaf: The transaction registers a handler (the "progressed" call), that fulfills instances of the request with the transaction (which contains the client).

I think you really want observables for multiple values - although I think the disposer pattern can also do it nicely.

Unfortunately, the good folks who maintain the promise specification and write libraries to support it don't know how genial they really are. The progress inteface works fine, and plays an irreplacable role in coordinating activity that would otherwise need the equivalent of "thread-local storage" to effect.

I think they do, kriskowal wrote https://github.com/kriskowal/gtor after watching channel9.msdn.com/Events/Lang-NEXT/Lang-NEXT-2014/Keynote-Duality - I think we realize that progression on promises is simply not a very sound abstraction. It's entirely possible to use it but there are better alternatives than promises for multiple-events and updates.

However, some users had too high expectations of progress: rather than have the root and leaf explicitly coordinate their interaction, as is encapsultated in the Request class, they wanted the promise library itself to mediate interactions. For instance, if the "progressed" handler throws an exception, perhaps the leaf should be automatically cancelled?

Right, and users expected that (inconsistently) and got really confused on many questions in StackOverflow when it didn't do what they want (tm). If the interaction does not need to be handled explicitly across the chain - passing a progress handler to the producer directly is a nice solution (as explained in the migration docs).

This sort of functionality is at odds with the purpose of the Promise interface, which is to simulate a synchronous flow of events accross asyncrhonous calls. In a "flow of events", surely a given thing should happen only once. Yet what if the progressed handler throws an exception but the leaf does something else? "What happens" at the leaf should be immutable, and the responsibility shouldn't be spread around, or have to be mediated by complex conventions.

Exactly.

If the progressed handler can't affect the chain, though, then can't the progress event simply be handled by a global event handler? Currently, "progress" is decrecated, with the recommendation that "progress" calls should be replaced by global events.

Not global events by any means - the recommendations is to make super local events that are bound to whatever produced the promise and not the promise itself. Like you said - a promise is an immutable value + time. It is not aware of progress flowing through it because it is a single value - only what produces the promise is aware of it - it's not conceptually a part of the promise.

What was overlooked, however, was:

1) Progress calls are scoped events, and the scope encodes the identity of the call stack, which is otherwise unidentifiable.

Indeed, this is a nice property that is solved in the ways I mentioned above - scoped events to a call stack namely are observables (promises are to callbacks as observables are to events).

2) It is completely unnecessary to involve the promise library in mediating the interaction between handler and leaf. If necessary, leaf and root can transfer ownership of the result explicitly, while always maintaining one clear source of authority for the result.

Which is part of why progression was deprecated. You're making really good points against progress here - if ownership is explicit, it should not affect the resolution state or propagate - why not just pass a callback to the producer?

I hope that the maintainers of promises, reading this, will recognize what a hidden gem the "progress" interface really is. In another important respect it allows asynchronous code to be written like synchronous code, but avoids the overhead of "fibers" (or what have you).

People do recognize it - but don't want to conflate promises (single value + time) with multiple events over time which is observables.

All that said - I'll definitely try out xwarp in a future project and give you more feedback after I've actually used it and understand the problem better - as far as I know (cc @petkaantonov ) the intention is to provide LTS to 2.x users so you can use the 2.x branch for progress events just fine. You're also very welcome to raise this argument to TC39 via the ESDiscuss mailing list where the EcmaScript committee will see it :)

bergus commented 9 years ago

Ah thanks @shaunc, I finally understood what you've wanted to use progression events for! A very interesting idea. I think a little example code would help to demonstrate your problem:

function thirdPartyCode() { // must not be modified - assume getResource to be injected
    return getResource().then(…);
}
// now the problem:
function getResource() {
    … // how do I know whether I was called during a transaction, and to which one I belong?
}
// when used inside user code like this:
transaction(function() {
    return somePromiseChain.then(thirdPartyCode).…
}); // could be Promise.using pattern or whatever

@benjamingr

domains/fibers/.bind/explicit context - choose one.

Yes, that's the current choice. However, bind and explicit context require cooperation by the third party, which often are not aware of this issue (and it's a hazzle to pass through everything). And we don't want to use domains or fibers when we can have promises, do we?

a promise is an immutable value + time. It is not aware of progress flowing through it

Yeah, if only everything was pure :-) I mean, cancellation already extends this strict view… (of course it's just as controversial).

Last but not least @shaunc have a look at kriskowal/gtor/issues/27. That idea provides information on whom a callback belongs to as well, do you think this could help here?

benjamingr commented 9 years ago

@spion where does getResource get the database handle itself from? If you control the DB handle passed to it (which is reasonable) you can pass a DB-with-context handle to it instead of a regular DB one

On Apr 6, 2015, at 17:05, Bergi notifications@github.com wrote:

Ah thanks @shaunc, I finally understood what you've wanted to use progression events for! A very interesting idea. I think a little example code would help to demonstrate your problem:

function thirdPartyCode() { // must not be modified - assume getResource to be injected return getResource().then(…); } // now the problem: function getResource() { … // how do I know whether I was called during a transaction, and to which one I belong? } // when used inside user code like this: transaction(function() { return somePromiseChain.then(thirdPartyCode).… }); // could be Promise.using pattern or whatever @benjamingr

domains/fibers/.bind/explicit context - choose one.

Yes, that's the current choice. However, bind and explicit context require cooperation by the third party, which often are not aware of this issue (and it's a hazzle to pass through everything). And we don't want to use domains or fibers when we can have promises, do we?

a promise is an immutable value + time. It is not aware of progress flowing through it

Yeah, if only everything was pure :-) I mean, cancellation already extends this strict view… (of course it's just as controversial).

Last but not least @shaunc have a look at kriskowal/gtor#27. That idea provides information on whom a callback belongs to as well, do you think this could help here?

— Reply to this email directly or view it on GitHub.

bergus commented 9 years ago

@benjamingr I think the point of the framework was that you don't need to control the handle and pass it explicitly any more. in the gist, this line:

Any utility called by view_foo -- even a plugin written by a third party which has no notion of transactions -- can get a database cursor:

does assume that for example. I think it could also simplify code a lot if you didn't have to use local db-with-context handles any more but could just use a global db reference. You could transparently wrap tasks in a transaction (or in whatever), without requiring anything special from the task functions.

benjamingr commented 9 years ago

@bergus I'm still not convinced this isn't better solved with DI - a container would resolve the database for you and it'd do so with context - this is typically how frameworks do it.

 plugin.use(function myPlugin(db){ // db is resolved by the framework to db_with_context, no wiring needed

 })
spion commented 9 years ago

@shaunc @bergus IMO the right solution here is definitely a different monadic (or promise-like) abstraction like the tx monad: compose the chain first, and provide the transaction/savepoint at the very end. I'll try and see if I can write a standalone module that supports both transactions and savepoints tonight