-
-
Notifications
You must be signed in to change notification settings - Fork 23
-
-
Notifications
You must be signed in to change notification settings - Fork 23
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
Update docs #2
Comments
Sorry about that. The docs were the original thought about making everything accessible on the |
@phated is this somehow related to you not being happy about async-time in this comment? I'm just going over all the Gulp4-related repos and trying to piece all the elements together. But during this process I'm asking myself the following question: should we time all the functions or just tasks? In other words: what is the relation of bach (which, if I understand correctly, is primarily concerned with functions composition / orchestration) and async-time. Maybe timing should just come to the picture on the higher (task) level? When it comes to settling parallel / series functions - I would be curious to know what were real-life scenarios where you guys thought of introducing them? I was naively thinking that we want to have the fail-fast semantics for build steps. Sorry if I'm asking stupid questions here - still wrapping my head over various pieces of the Gulp4 puzzle... |
@pkozlowski-opensource you are correct, I am not happen with the latest bach API due to async-time needing bach to be an event emitter. The need for timing should only be needed at the task level, but then it would have to happen inside The scenario for settling is partial builds. In gulp, people have requested that if one task fails, not to fail the other tasks. Settling allows all tasks to complete (series or parallel) before reporting the errors that occurred. Also, it is a pattern that is seen in promise libraries but not in async libraries I have looked at. By exposing it in bach, gulp/orchestrator can utilize a settle function if a partial build option is set. The questions aren't stupid and I am glad someone cares about the design decisions here, as most people have ignored the work towards gulp4. |
IMO a possible solution is to make bach always an EventEmitter, by shifting all the functions to the main index.js and always timing everything. |
@D10 we won't be timing everything unless it is unobtrusive. Timing currently adds 1 or 2 closures around the user supplied function to do the timing. |
FWIW, the more I'm looking into Gulp4 related repos the more I in favour of moving all the timing-related responsibilities out of bach. I kind of like the idea of bach being just concerned with sequences / parallel execution of "async" functions. @phated I would be interested in hearing more about why putting those responsibilities on the orchestrator level seems weird to you. Is it the fact that we would have to wrap timing functionality around each argument passed to series / parallel? What would be a practical concern here? |
@pkozlowski-opensource I actually remembered why we can't do it in the orchestrator tier, and that is because you don't have access to the completion of a single task. Since a task can end in a variety of ways and that is handled by async-done, there is no easy way to wrap a task function to know when it finished in any way. My initial implementation passed a callback function in, but tasks that returned streams were never finished since they didn't call that timing callback. |
@pkozlowski-opensource i am 100% open to having orchestrator be the timing tier and simplifying bach once again, but I am at a loss for how to do it. All thoughts welcome. |
@phated I'm still (very) new to those things but assuming that we want to remove all the timing thing from bach and move it to the orchestrator level, couldn't we wrap tasks / private functions:
I was experimenting with the code in this commit: pkozlowski-opensource/orchestrator@7ad302f and it seems to work but I'm sure I'm missing 10 000 different things here... The one thing that is missing for sure is the ability to specify a name to async-time instead of always letting it figure out the name from the function name (so we can provide a task name for an anonymous task function). Anyway, sharing what I had on my mind - even if stupid maybe it will trigger some discussion / other ideas... |
the problem I have with wrapping with async-time inside orchestrator is that async-time wraps async-done and bach uses async-done, so we have at least 3 layers of closures to get timing and those create extra domains, etc. |
@phated right, those multiple layers of wrapping are valid concern. I just wonder if this isn't natural conflict between trying to keep all the elements of the stack (bach, async-done, async-time etc.) generic and re-usable with any async functions. I mean, if we would do the async-time wrapping on the orchestrator level we would know that every function that goes to layers below (bach) is callback-based. So we could collapse all those wrapping layers by ... not using async-done in bach, but it would obviously make it (almost) a proxy to async (and limit it general aspect) :-/ At the moment it seems to me like there is a tension between building on top of generic building blocks vs. having minimal code / operations - which is not uncommon in programming :-) I'm not very familiar with domain and still quite new to this whole node-fu so not sure what are practical implications of having those 3 layers of closures / domains in the context of gulp / orchestrator? |
@pkozlowski-opensource I think I solved the abstraction problem. The thing that was the most restricting was using |
@phated cool! I gave it a try and the whole thing is easy to embed in the orchestrator (pkozlowski-opensource/orchestrator@3a431a0). Also adding timing is easy: https://github.com/pkozlowski-opensource/orchestrator/blob/time_now_and_later/index.js so all looks good here. I've also combined everything into a working POC: https://github.com/pkozlowski-opensource/gulp4-poc/tree/now_and_later_time Now, while playing with those various pieces I've noticed a couple of things. Method signatures
Orchestrator's repository cleanup
Naming
After looking into object-notation some more this might be remedy for naming problems, need to think about it some more. |
@phated in short: this all looks very promising: simple and I can see it working. Still there are couple of things to clarify:
I can start opening individual issues for various repos / tackling various items as soon as we agree on the above. I would really love to see the new task system in place sooner than later so willing to put effort here. |
I just updated now-and-later to be a little more streamlined and configurable. I want it to exist as the layer underneath bach. bach will be the composition layer that returns a closure, which reduces overhead for anyone wanting to use now-and-later in place of async.map/mapSeries (since the API can be used in the exact same way now). |
The other thing I added was a |
I might remove the dependency on async-done and move that into bach. Thoughts? |
Good news! |
The docs mention that settle{Series,Parallel} can be accessed on the bach object, but AFAIK they are only accessible via require("bach/settle"). Perhaps you should update the docs about this.
The text was updated successfully, but these errors were encountered: