-
-
Notifications
You must be signed in to change notification settings - Fork 625
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
Recompile task after runDeps to reload envs set by deps #1746
base: main
Are you sure you want to change the base?
Recompile task after runDeps to reload envs set by deps #1746
Conversation
0b20436
to
0966a0a
Compare
@andreynering @pd93 If this approach doesn't seem optimal, I am considering setting up a custom system to reload dotenv and execute the tasks. I would appreciate your thoughts on whether this might be a viable direction. |
By the way, currently, |
@andreynering @pd93 What do you think about this pull request? |
fmmmmm..... |
Hey @hori-ryota, I know we've been slow on reviewing pull requests. We all have full time jobs and a real life happening. I added myself as a reviewer to remember to take a look when possible. |
Thank you! Please let me know if there's anything else I should do or improve. I really appreciate this excellent tool! |
@hori-ryota In the meantime, can you rebase this PR with |
0966a0a
to
90fd17b
Compare
rebased. |
if len(t.Deps) > 0 { | ||
// Recompile the task after running dependencies to reload environment variables from dotenv files which may have been set up during the dependency execution | ||
t, err = e.CompiledTask(call) | ||
if err != nil { | ||
return err | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This introduces a small performance penalty, and is something that is not needed 99% of the time.
I'm not so sure that it's worth doing for this rare edge case. In this case, it could be better to just run both tasks separately so you ensure that the .env
file exists before running the actual task.
task setup && task actual-task
I suppose that your .env
should be generated only once, right? I wouldn't expect a .env
file to have to be generated on every run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your feedback. In a large monorepo like ours, each module has its own Taskfile and setup. Being able to run a single task and have all necessary setups execute via dependencies greatly simplifies our workflow. Without this, we’d need to run separate setup tasks for each module, making management and documentation more complicated.
Additionally, our .env requirements change frequently as the team iterates. Running a lightweight, idempotent setup every time ensures our environment is always up-to-date and stable, reducing errors and confusion in the long run. This approach helps keep the entire project’s operations more efficient and maintainable.
fix #1381
I would like to create an environment variable file using deps, read it with dotenv, and use it in cmds.
tested by followings
Taskfile.test.yml
(copy from dotenv files are loaded before task dependencies are ready #1381 )rm -f .env && go run ./cmd/task -t Taskfile.test.yml my-task
result:
(before):
(after):