-
-
Notifications
You must be signed in to change notification settings - Fork 62
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
Included task envfile doesn't consider cwd #160
Comments
(I thought I found that the issue was with a virtualenv, hence closing, but it actually seems not.) |
Hi @rjaduthie, thanks for raising this! I see how that could be surprising. If I understand correctly what you're demonstrating then I guess the solution would be to make this line do something a bit like this one. You want to have go at it? One complication with this however is that one might indeed wish for included tasks to be executed in a different directory, but still manage all the local (non checked in) .env files in the project root. So it be nice if we could work out how to have it both ways. Some ideas:
|
Thanks for the hints. I'll try to take a look at this at some point and suggest a PR to fix. In a follow up, I think there's a similar effect with the specification of commands as well. Using the example from my original post, if the subproject task uses a
... here the second task within the sequence is within a subdir of the subproject, but Poe doesn't "include" it with the correct cwd. |
To respond to your question about the design options: I wonder how obvious/intuitive it could be to a user if implementing the first option. The second option could also get tricky, if you want to override an inherited behaviour. A third option might be to have a flag within a task or the include block that specifies which behaviour to use - e.g. |
@rjaduthie FYI I've started looking into this. It's more complex than I realised and I think justifies some refactor of how tasks manage and pass certain types of configuration. |
Also: - apply correction if cwd value passed to app is a file path - begin refactor of how configuration is managed across tasks - improve error handling
Also: - apply correction if cwd value passed to app is a file path - begin refactor of how configuration is managed across tasks - improve error handling
Also: - apply correction if cwd value passed to app is a file path - begin refactor of how configuration is managed across tasks - improve error handling
The issue with sequence task members inheriting cwd should be fixed now with v0.24.2 I'm still working on a fix for the issue included task files which involves a larger refactor. |
Just to say, we started using the updated package and it now works well for setting the working directory in sequenced tasks. Thank you for that fix! |
The 0.26.0 release includes a minor breaking change that addresses the original issue. The directory specified by the cwd option when including config will now be used at the base path for resolving relative paths for envfiles referenced within the included file. The docs have been updated to reflect this. |
Summary
Circumstances
When including a task from another file definition - e.g.
./subproject/pyproject.toml
- and using thecwd
property in the rootpyproject.toml
.Effect
If an
envfile
is used in the task definition in the submodule project, the path to the envfile doesn't consider thecwd
when it's imported.To recreate
Assuming there's an executable
my_task
and an env file./subproject/.env.my_task
:./pyproject.toml
:./subproject/pyproject.toml
Execution of
poetry run poe my_task
in the root project will have the variables as defined in./subproject/.env.my_task
.Possible solutions
I've had a bit of a look around the code. Possibly when adding envs to the
EnvVarsManager
, thecwd
can be respected forinclude
'd tasks?The code is quite complex (... I haven't yet worked out the interaction with the
_get_upstream_invocations
etc.), so I haven't got a PR for this issue. I might have so time to look later, but hope that I can get some pointers to how to approach this fix.The text was updated successfully, but these errors were encountered: