Skip to content

SDK-376: SDK should prompt for database uri again if an invalid uri was entered#325

Open
mherman22 wants to merge 4 commits intoopenmrs:masterfrom
mherman22:SDK-368-followup
Open

SDK-376: SDK should prompt for database uri again if an invalid uri was entered#325
mherman22 wants to merge 4 commits intoopenmrs:masterfrom
mherman22:SDK-368-followup

Conversation

@mherman22
Copy link
Member

@mherman22 mherman22 commented Jan 15, 2025

Description of what I changed

Issue I worked on

see https://openmrs.atlassian.net/browse/SDK-376

Checklist: I completed these to help reviewers :)

  • My IDE is configured to follow the code style of this project.

    No? Unsure? -> configure your IDE, format the code and add the changes with git add . && git commit --amend

  • I have added tests to cover my changes. (If you refactored
    existing code that was well tested you do not have to add tests)

    No? -> write tests and add them to this commit git add . && git commit --amend

  • I ran mvn clean install right before creating this pull request and
    added all formatting changes to my commit.

    No? -> execute the above command

  • All new and existing tests passed.

    No? -> figure out why and add the fix to your commit. It is your responsibility to make sure your code works.

  • My pull request is based on the latest changes of the master branch.

    No? Unsure? -> execute command git pull --rebase upstream master

@mherman22 mherman22 changed the title SDK-368: SDK should prompt for database uri again if an invalid uri was entered SDK-376: SDK should prompt for database uri again if an invalid uri was entered Jan 15, 2025
Copy link
Member

@wikumChamith wikumChamith left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@mseaton mseaton left a comment

Choose a reason for hiding this comment

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

Would be nice to see some tests around this if possible. Otherwise looks ok.

@wikumChamith
Copy link
Member

wikumChamith commented Jan 16, 2025

@mherman22 does this work if I pass invalid credentials using this command:

org.openmrs.maven.plugins:openmrs-sdk-maven-plugin:6.1.0-SNAPSHOT:setup -DdbUri=jdbc:mysql://localhost:3306/@DBNAME@ -DdbDriver=mysql -DdbUser=root -DdbPassword=wrongPassword

@mherman22
Copy link
Member Author

mherman22 commented Jan 16, 2025

@mherman22 does this work if I pass invalid credentials using this command:

org.openmrs.maven.plugins:openmrs-sdk-maven-plugin:6.1.0-SNAPSHOT:setup -DdbUri=jdbc:mysql://localhost:3306/@DBNAME@ -DdbDriver=mysql -DdbUser=root -DdbPassword=wrongPassword

Fails with [ERROR] Failed to execute goal org.openmrs.maven.plugins:openmrs-sdk-maven-plugin:6.1.0-SNAPSHOT:setup (default-cli) on project openmrs-sdk: Failed to setup server: Failed to connect to database after 3 attempts. Please verify your credentials and try again. Invalid database credentials. Please check your username and password. Access denied for user 'root'@'localhost' (using password: YES) -> [Help 1]

And with that error, it should prompt the user for new credentials until they get it right(limitd to 3 attempts

@wikumChamith
Copy link
Member

@mherman22 does this work if I pass invalid credentials using this command:
org.openmrs.maven.plugins:openmrs-sdk-maven-plugin:6.1.0-SNAPSHOT:setup -DdbUri=jdbc:mysql://localhost:3306/@DBNAME@ -DdbDriver=mysql -DdbUser=root -DdbPassword=wrongPassword

Fails with [ERROR] Failed to execute goal org.openmrs.maven.plugins:openmrs-sdk-maven-plugin:6.1.0-SNAPSHOT:setup (default-cli) on project openmrs-sdk: Failed to setup server: Failed to connect to database after 3 attempts. Please verify your credentials and try again. Invalid database credentials. Please check your username and password. Access denied for user 'root'@'localhost' (using password: YES) -> [Help 1]

And with that error, it should prompt the user for new credentials until they get it right(limitd to 3 attempts

Yeah. It would be better if we could avoid the whole error message and just output something like the following:

Database connection failed (attempt 1 of 3): Invalid database credentials. Please check your username and password.

@mherman22 mherman22 force-pushed the SDK-368-followup branch 2 times, most recently from d91c270 to 1751f0f Compare February 2, 2025 21:25
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.

3 participants