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

Have downloads always end up under WORKING_DIRECTORY #5913

Merged
merged 1 commit into from
Oct 22, 2024

Conversation

gerrod3
Copy link
Contributor

@gerrod3 gerrod3 commented Oct 15, 2024

fixes: #5912

Comment on lines 131 to 138
work_dir = str(settings.WORKING_DIRECTORY)
self._writer = tempfile.NamedTemporaryFile(
dir="." if work_dir in os.getcwd() else work_dir,
suffix=suffix,
delete=False,
)
Copy link
Member

Choose a reason for hiding this comment

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

It appears like this is a better variant of what I was trying to do in the past: pulp/pulp_container#1299 (comment).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'm doing something similar with my new work on pull-through caching in pulp-python and I thought I should just move the fix into pulpcore.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm this feels like patching over a deeper issue. But well.
I think we should leave a comment on why this is important and the assumptions that lead to there.

Comment on lines 131 to 138
work_dir = str(settings.WORKING_DIRECTORY)
self._writer = tempfile.NamedTemporaryFile(
dir="." if work_dir in os.getcwd() else work_dir,
suffix=suffix,
delete=False,
)
Copy link
Member

Choose a reason for hiding this comment

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

Hmm this feels like patching over a deeper issue. But well.
I think we should leave a comment on why this is important and the assumptions that lead to there.

self._writer = tempfile.NamedTemporaryFile(dir=".", suffix=suffix, delete=False)
work_dir = str(settings.WORKING_DIRECTORY)
self._writer = tempfile.NamedTemporaryFile(
dir="." if work_dir in os.getcwd() else work_dir,
Copy link
Member

Choose a reason for hiding this comment

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

This is a simple string comparison. So at least it should be os.getcwd().startswith(work_dir). But i think i'd prefer using pathlib at this point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, updated to use pathlib logic and added a simple comment for the check.

@mdellweg mdellweg merged commit e926a69 into pulp:main Oct 22, 2024
12 checks passed
Copy link

patchback bot commented Oct 22, 2024

Backport to 3.49: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.49/e926a69f42ce9a5e546f7d66463bda897aec27cb/pr-5913

Backported as #5926

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Downloaders fail when current directory is not inside WORKING_DIRECTORY
3 participants