Refactor tasks to not fork, and always run in-process of the main execution pipeline #1613
Conversation
… inside the same pipeline as tasks provided by plugins.
…aces in their task name
Deploying with
|
| Latest commit: |
b405eb0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://137fd0cf.taqueria.pages.dev |
| Branch Preview URL: | https://refactor-internal-tasks.taqueria.pages.dev |
mweichert
changed the title
|
Note, I've created an issue for something discovered in this effort of work: #1614 |
|
| option === OPT_IN | ||
| ? 'The command "taq opt-in" is ignored as this might be the first time running Taqueria...' | ||
| : 'The command "taq opt-out" is ignored as this might be the first time running Taqueria...' | ||
| mapRej(previous => |
There was a problem hiding this comment.
Do we still need it? I thought this would not be needed after the internal task refactor
There was a problem hiding this comment.
I'm not sure to be honest. If not, I assume we'll address that in a different ticket/PR.
The acceptance criteria for this particular issue/PR was:
Refactor remaining internal tasks so that the execution flow matches that of plugin-provided tasks
As that criteria is a little vague, I interpreted it to be:
Refactor tasks to not fork but run in the main execution pipeline
The motive for that , as far as I was aware, was to resolve a) memory leaks; b) concurrency issues; c) make the code easier to reason about; d) ability to ask for --help for built-in tasks
I had thought that the only thing holding back GA tracking was issue B above.
--
I modified the following code strictly for type correctness, as this section of code was returning Future<string, unknown> instead of Future<TaqError, unknown>:
Before:
mapRej(() =>
option === OPT_IN
? 'The command "taq opt-in" is ignored as this might be the first time running Taqueria...'
: 'The command "taq opt-out" is ignored as this might be the first time running Taqueria...'
)
After:
mapRej(previous => TaqError.create({
kind: 'E_OPT_IN_WARNING',
msg: option === OPT_IN
? 'The command "taq opt-in" is ignored as this might be the first time running Taqueria...'
: 'The command "taq opt-out" is ignored as this might be the first time running Taqueria...',
previous,
context: option,
})
If the above code isn't needed anymore, great - but I'd probably prefer making that adjustment in a different PR, as I cannot recall the reason why Jimmy added this code, and too, I think that's outside of the scope I had in mind in this PR. Would you be fine if we created an issue for this instead, Ali? I would do so, but given that you recognized this, I assume you might have more context/recollection about why this code was here to start with.
|
Is the |
@danielelisi Main was merged into this branch 2 days ago: c1d4a33 Feel free to modify this branch directly if something looks off with regards to the workflow stuff. |
🌮 Taqueria PR - Targeting prerelease branch.
🪁 Description
We have three groups of tasks:
Global tasks
Internal tasks
Plugin-provided tasks
Before this PR, global tasks and internal tasks would get executed outside of the main execution pipeline (future).
This has created concurrency issues, and been the source for many memory leaks.
🪂 Pre-Merge Checklist (Definition of Done)
🚦 Required to merge:
🛩️ Summary of Changes
This PR adjusts the CLI in such that:
This results in fewer memory leaks, and resolves some concurrency/timing issues that was blocking other work.
This branch also fixes #309
🎢 Test Plan
I've manually tested each global and internal task.