Skip to content

Fix temp file leak in pull-through metadata streaming. - #7933

Draft
aKlimau wants to merge 1 commit into
pulp:mainfrom
aKlimau:pull-through-leak
Draft

Fix temp file leak in pull-through metadata streaming.#7933
aKlimau wants to merge 1 commit into
pulp:mainfrom
aKlimau:pull-through-leak

Conversation

@aKlimau

@aKlimau aKlimau commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Currently there is a problem with non content type files staying indefinitely on the disk when using _stream_remote_artifact() even if save_artifact flag is set to false.

I don't see a good reason for writing this data on disk in the first place, this pr fixes that.

fixes: #7846

📜 Checklist

  • Commits are cleanly separated with meaningful messages (simple features and bug fixes should be squashed to one commit)
  • A changelog entry or entries has been added for any significant changes
  • Follows the Pulp policy on AI Usage
  • (For new features) - User documentation and test coverage has been added

See: Pull Request Walkthrough

await response.write(data)
if remote.policy != Remote.STREAMED:
if save_artifact and remote.policy != Remote.STREAMED:
await original_handle_data(data)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is where the downloader saves the file to disk?
At the very least this change looks reasonable to me.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It looks like there are several places where we check for exactly save_artifact and remote.policy != Remote.STREAMED can we redefine the variable so that it already contains the streamed condition?

(Somehow this may be a copy-and-paste / missed-all-the-places-to-update bug coming from exactly there.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is where the downloader saves the file to disk?

Yes, original_handle_data that usually points to BasedDownloader handle_data that does the writing (if not overriden by plugins downloader)

@aKlimau
aKlimau force-pushed the pull-through-leak branch 2 times, most recently from bde34c6 to 2ab7b43 Compare July 29, 2026 10:23
@github-actions github-actions Bot removed the no-issue label Jul 29, 2026
@aKlimau
aKlimau force-pushed the pull-through-leak branch from 2ab7b43 to 1011edc Compare July 29, 2026 13:16
@dralley
dralley marked this pull request as draft July 29, 2026 16:32
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.

Pull-through content app never deletes the temp file for non-content paths (fills WORKING_DIRECTORY)

2 participants