Make it clear the settings need to be applied after restoring to an unconfigured instance #381
Conversation
|
|
||
| if ! $cluster; then | ||
| echo "Visit https://$hostname/setup/settings to review appliance configuration." | ||
| echo "Visit https://$hostname/setup/settings to review appliance configuration. Settings must be applied to complete any necessary migrations." |
There was a problem hiding this comment.
How about:
You must now visit https://$hostname/setup/settings to review and save appliance configuration
I feel like this is giving a direct command to the user and actually mentions 'saving' the configuration. Users who are unfamiliar with GHE may not know that hitting 'Save settings' is the equivalent of 'applying settings'?
There was a problem hiding this comment.
I agree we should probably be a little firmer here, though "you must" feels a little odd to me.
How about:
Please visit https://$hostname/setup/settings to review and save the appliance configuration.
There was a problem hiding this comment.
I still feel that sounds almost optional though… How about:
To complete the restore process, please visit https://$hostname/setup/settings to review and save the appliance configuration.
Then at least we're giving people the reason for doing this.
There was a problem hiding this comment.
Hahah words are hard. Hesitated making this change due to the repetition of 'complete' (previous output says it's complete, now I need to do something to complete it). If neither of you think it's an issue I'll go ahead with it :)
There was a problem hiding this comment.
Good point @keeran. How about changing "Completed" in the line above to "Finished", so users will see:
Finished restore of 5.5.5.5:122 from snapshot 20180326T020444
To complete the restore process, please visit https://5.5.5.5/setup/settings to review and save the appliance configuration.
Or move to the end of the line:
Restore of 5.5.5.5:122 from snapshot 20180326T020444 finished.
To complete the restore process, please visit https://5.5.5.5/setup/settings to review and save the appliance configuration.
|
|
||
| if ! $cluster; then | ||
| echo "Visit https://$hostname/setup/settings to review appliance configuration." | ||
| echo "To complete the restore process, please visit https://$hostname/setup/settings to review and save the appliance configuration." |
There was a problem hiding this comment.
It should only be necessary to do this if you are restoring on to an unconfirmed appliance, as we automatically run ghe-config-apply on configured appliances at the end of the restore process:
Lines 400 to 413 in 7d0ca70
Maybe the if could be changed to if ! $instance_configured; then so that this message only appears on un-configured appliances.
There was a problem hiding this comment.
@keeran don't forget this 😉 (I'd like to merge this before merging by big PR so it makes it in before I create the legacy branch).
| fi | ||
|
|
||
| echo "Completed restore of $GHE_HOSTNAME from snapshot $GHE_RESTORE_SNAPSHOT" | ||
| echo "Finished restore of $GHE_HOSTNAME from snapshot $GHE_RESTORE_SNAPSHOT" |
There was a problem hiding this comment.
Re-reading this, I think we should go with the second wording as per #381 (comment)
lildude
changed the title
…t-always-restore-column-encryption-keys Backport 349 for 3.9: Always restore column encryption keys
4 participants
Following a restore we should prompt the user to save settings on the appliance as this will trigger any necessary migrations.