Skip to content
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

feat(remote): global tempDir when the path is absolute #1661

Merged
merged 9 commits into from
Jun 28, 2024

Conversation

vmaerten
Copy link
Member

@vmaerten vmaerten commented May 16, 2024

This PR allow to have a global temp directory when using remote taskfile.

Considering a setup with multiple different projects, all using the same remote taskfile. if / when the remote taskfile changes, we need to approve the prompt for all projects.
At the beginning, I thought changing the TASK_X_TEMP_DIR to ~/.task, but I've seen that it's suffixed with the dir so I won't work.

The fingerprint dir should stay scoped

This PR changes the behavior only for remote. If the path is absolute or start with ~ then we do not suffix it.

For the implementation, I've chosen to create a struct and store the two paths in it. We could also in the future add an env variable like TASK_X_REMOTE_DIR. If you think it can be a nice addition, I can implement it in this PR

Any feedback on this feature are appreciated :)

@vmaerten vmaerten requested review from pd93 and andreynering May 16, 2024 18:51
Copy link
Member

@andreynering andreynering left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vmaerten Good catch, this makes sense! The implementation looks good to me as well.

I think a TASK_REMODE_DIR would be a good addition to this change. If given, use that, otherwise fallback to TASK_TEMP_DIR as you already did.

@vmaerten
Copy link
Member Author

@andreynering thanks! I've added the new env variable.
For now I did not put it in the Environment Reference doc section because it's linked to the experimentation but I've added a small section in caching

Let me know if you think that adding it in environment reference is needed.

It would be amazing if we could get this merged before the next version

@vmaerten vmaerten requested a review from andreynering June 14, 2024 08:31
Copy link
Member

@andreynering andreynering left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if you think that adding it in environment reference is needed.

@vmaerten Yes, please add to this section: https://taskfile.dev/api/#env

website/docs/experiments/remote_taskfiles.mdx Outdated Show resolved Hide resolved
vmaerten and others added 2 commits June 28, 2024 09:43
Co-authored-by: Andrey Nering <andrey@nering.com.br>
@vmaerten
Copy link
Member Author

Done ! https://deploy-preview-1661--taskfile.netlify.app/next/reference/environment
image

@pd93 pd93 merged commit 830b745 into go-task:main Jun 28, 2024
13 checks passed
pd93 added a commit that referenced this pull request Jun 28, 2024
@vmaerten vmaerten deleted the feat/remote-global-temp branch July 7, 2024 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants