Skip to content
This repository has been archived by the owner on Sep 8, 2024. It is now read-only.

Replace hardcoded use of /tmp directory #2892

Merged
merged 17 commits into from
May 7, 2021
Merged

Conversation

krisgesling
Copy link
Contributor

Description

This adds a new method get_temp_path() that should be used anytime a temporary file or directory is needed. This replaces all uses of the hard coded /tmp directory, in favor of the cross-system tempfile.gettempdir()

The new method does not create any directories or files as it has no concept of whether the requested path contains a file or not.

Fixes #2727
Replaces #2740

All credit to @dzekem who identified the issue and did the bulk of the work on this.

How to test

Unit tests included.
This also touches many parts of the code base.

Contributor license agreement signed?

@krisgesling krisgesling added Type: Enhancement - proposed New proposal for a feature that is not currently a priority on the roadmap. Status: To be reviewed Concept accepted and PR has sufficient information for full review labels Apr 30, 2021
@pep8speaks
Copy link

pep8speaks commented Apr 30, 2021

Hello @krisgesling! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-05-07 06:04:55 UTC

@devops-mycroft devops-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Apr 30, 2021
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

1 similar comment
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

forslund
forslund previously approved these changes Apr 30, 2021
Copy link
Collaborator

@forslund forslund left a comment

Choose a reason for hiding this comment

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

Added a single nit-picky comment, generally it looks good. Looking forward to getting this into core!

@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

Copy link
Collaborator

@forslund forslund left a comment

Choose a reason for hiding this comment

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

Looks super nice now!

@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

@krisgesling krisgesling merged commit 088f563 into dev May 7, 2021
@krisgesling krisgesling deleted the bugfix/consistent-temp-dir branch May 7, 2021 21:58
@PureTryOut PureTryOut mentioned this pull request Jun 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) Status: To be reviewed Concept accepted and PR has sufficient information for full review Type: Enhancement - proposed New proposal for a feature that is not currently a priority on the roadmap.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replacing hard coded /tmp/ for some instances in the code
5 participants