Skip to content

spline #1191 Admin: print a backup reminder before database migration#1195

Merged
wajda merged 4 commits into
developfrom
feature/spline-1191-admin-backup-reminder
Apr 17, 2023
Merged

spline #1191 Admin: print a backup reminder before database migration#1195
wajda merged 4 commits into
developfrom
feature/spline-1191-admin-backup-reminder

Conversation

@wajda
Copy link
Copy Markdown
Contributor

@wajda wajda commented Mar 22, 2023

Fixes #1191

Result:

image

@wajda wajda requested a review from cerveada as a code owner March 22, 2023 16:19
@wajda wajda requested a review from korbelm March 22, 2023 16:19
@cerveada
Copy link
Copy Markdown
Contributor

What if the upgrade is run via script, not manually, will this cause issue in such a case? Should there be some -y option to agree automatically?

@wajda
Copy link
Copy Markdown
Contributor Author

wajda commented Mar 23, 2023

What if the upgrade is run via script, not manually, will this cause issue in such a case? Should there be some -y option to agree automatically?

All the user interactions are enabled when there is a console. In scripts, or e.g. when you run it from IDEA etc when the console is not attached to the process the above message won't appear.
The main intention for this feature is just to remind the user about the necessity of database backup. In script it would be too late anyway.

@wajda
Copy link
Copy Markdown
Contributor Author

wajda commented Mar 23, 2023

@korbelm , do you think we should print it even in the non-interactive mode, effectively always requiring -y option in addition to db-upgrade while running it from scripts?

@korbelm
Copy link
Copy Markdown

korbelm commented Mar 24, 2023

i would keep -y even for automated run (other sw do) as it should make users more aware of potential risks, from my point of view there is no need to have separate behaviour

@wajda wajda marked this pull request as draft April 14, 2023 13:25
@wajda wajda force-pushed the feature/spline-1191-admin-backup-reminder branch from 187045b to 245332a Compare April 14, 2023 13:29
wajda added 2 commits April 15, 2023 03:48
…lag name was changed to '--non-interactive' instead of '-y' for clarity.
@sonarqubecloud
Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@wajda
Copy link
Copy Markdown
Contributor Author

wajda commented Apr 15, 2023

Added the --non-interactive flag suggested above. I didn't use -y as it implies "yes", but there are also other prompts in the Admin CLI, not just yes/no.

The updated logic is the following:

  • If the --non-interactive flag is used, regardless of the console availability the process just carries on after printing the corresponding warning message.

image

  • If the --non-interactive is NOT used then the behavior depends on the console availability. If it's available, the command acts interactively

image

  • otherwise it prints an error message instructing the user to switch into the non-interactive mode.

image

@wajda wajda marked this pull request as ready for review April 15, 2023 10:18
@wajda wajda merged commit 3cd6c9f into develop Apr 17, 2023
@wajda wajda deleted the feature/spline-1191-admin-backup-reminder branch April 17, 2023 21:28
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.

Admin: print a backup reminder before database migration

3 participants