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

Backup configuration files before overwriting #11216

Merged
merged 2 commits into from Dec 19, 2017

Conversation

cgarwood
Copy link
Member

@cgarwood cgarwood commented Dec 18, 2017

Description:

Creates a backup copy of the default configuration files (secrets.yaml, automation.yaml, customize.yaml, etc) before overwriting/re-creating them if configuration.yaml is re-created.

Related issue (if applicable): fixes #11213, #10190

Checklist:

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass

@cgarwood cgarwood requested a review from a team as a code owner December 18, 2017 19:56
@fabaff
Copy link
Member

fabaff commented Dec 19, 2017

Covers #10190, too.

if os.path.isfile(secret_path):
backup_secret_path = "{}.{}.bak".format(
secret_path,
int(date_util.as_timestamp(date_util.now()))
Copy link
Member

Choose a reason for hiding this comment

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

For humans it would be easier to read if we are not use epoch but an ISO 8601 timestamp.

@cgarwood
Copy link
Member Author

Updated to ISO 8601 format without the symbols, so 20171219T104501 instead of 2017-12-19T10:45:01 since colons aren't permitted in filenames on some filesystems.

Copy link
Member

@fabaff fabaff left a comment

Choose a reason for hiding this comment

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

Thanks 🐦

@fabaff fabaff merged commit 90e25a6 into home-assistant:dev Dec 19, 2017
@cgarwood cgarwood deleted the dont_overwrite_config branch December 19, 2017 19:08
Copy link
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Good! A future PR could extract the added logic into a small function that takes the path to the file in question and timestamp and performs the print and rename.

@amelchio
Copy link
Contributor

Rather than littering the configuration directory, I have a feeling that it could be better to just detect the partial configuration, exit and let the user handle the conflict. It's not a particularly strong feeling, though :)

@pvizeli
Copy link
Member

pvizeli commented Dec 21, 2017

I feel also like @amelchio. Let's only add a warning and user know that the file was not new created. I revert this change now, for this it need a core review. It use also prints instead logs.

pvizeli added a commit that referenced this pull request Dec 21, 2017
pvizeli added a commit that referenced this pull request Dec 21, 2017
* Revert "Adding MotionIP to BinarySensors for HMIP-SMI (#11268)"

This reverts commit c94cc34.

* Revert "Bugfix: 10509 - http is hard coded in plex sensor (#11072)"

This reverts commit 901d4b5.

* Revert "Fix handling zero values for state_on/state_off (#11264)"

This reverts commit 2e4e3a4.

* Revert "Fix inverted sensors on the concord232 binary sensor component (#11261)"

This reverts commit b866687.

* Revert "Proper Steam game names and small fixes (#11182)"

This reverts commit 7faa940.

* Revert "Bugfix homematic available modus (#11256)"

This reverts commit 1d57958.

* Revert "Fix detection of if a negative node is in use (#11255)"

This reverts commit b28bfad.

* Revert "added myself to become code owner for miflora and plant (#11251)"

This reverts commit e068204.

* Revert "Add workaround for running tox on Windows platforms (#11188)"

This reverts commit 81f1a65.

* Revert "Upgrade to new miflora version 0.2.0 (#11250)"

This reverts commit 8efc4b5.

* Revert "homematic: add username and password to interface config schema (#11214)"

This reverts commit b4e2537.

* Revert "Backup configuration files before overwriting (#11216)"

This reverts commit 90e25a6.
@cgarwood
Copy link
Member Author

So how do we want to handle it? Leave the existing files in place and not create new ones? It's not a function that gets run often (I've only ever seen it used on initial launch, and if someone wants to troubleshoot with a blank config), so I don't think there will be an issue of it littering the config directory.

@balloob balloob mentioned this pull request Jan 11, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Mar 30, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default config (test) - Wipes out existing secrets.yaml
7 participants