Skip to content

Conversation

lstein
Copy link
Collaborator

@lstein lstein commented Apr 19, 2024

Summary

If the web app exits unexpectedly it may not run its shutdown routine, which removes temporary directories, among other things. At initialization time, this PR checks the base directories used by ObjectSerializerDisk for temporary directories left dangling from previous runs, and removes them.

Related Issues / Discussions

See #6242 for background on a bug that led to a large number of dangling temporary directories in outputs/tensors.

QA Instructions

If you have been running earlier versions of invokeai-web you probably have a lot of dangling temporary directories in outputs/tensors. When you run this PR, these directories should be removed.

Merge Plan

Merge when approved.

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Tests added / updated (if applicable)
  • Documentation added / updated (if applicable)

@psychedelicious
Copy link
Collaborator

My first thought is this should instead be the responsibility of the service that writes the files

@lstein
Copy link
Collaborator Author

lstein commented Apr 22, 2024

My first thought is this should instead be the responsibility of the service that writes the files

Good idea. I'll modify the PR.

@github-actions github-actions bot added the services PRs that change app services label Apr 23, 2024
@lstein
Copy link
Collaborator Author

lstein commented Apr 23, 2024

@psychedelicious The dangling tmpfile cleanup code has been moved into object_serializer_disk.py. When I did this I discovered many dangling tmp directories in outputs/conditioning as well as outputs/tensors, so this was a good move.

@lstein lstein force-pushed the lstein/feat/remove-dangling-tensor-tmpdirs branch from 9d8e0af to 1dc9c08 Compare April 23, 2024 01:52
@github-actions github-actions bot added the python-tests PRs that change python tests label Apr 23, 2024
Copy link
Collaborator

@psychedelicious psychedelicious left a comment

Choose a reason for hiding this comment

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

Thanks! I did a bit of tidying. There was a logic bug - start() is only called after init, so this would actually delete the tmp dir that was created for the current session. I moved the logic around to handle this.

Also, we shouldn't delete files when ephemeral=False, so I handled that.

Added tests.

@psychedelicious psychedelicious force-pushed the lstein/feat/remove-dangling-tensor-tmpdirs branch from a1fc349 to 8f8514d Compare April 23, 2024 07:02
@psychedelicious psychedelicious enabled auto-merge (rebase) April 23, 2024 07:02
@psychedelicious psychedelicious merged commit d7b5ad0 into main Apr 23, 2024
@psychedelicious psychedelicious deleted the lstein/feat/remove-dangling-tensor-tmpdirs branch April 23, 2024 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api python PRs that change python files python-tests PRs that change python tests services PRs that change app services
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants