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

Adding an error if the server path contains a space. #245

Merged
merged 1 commit into from
Aug 16, 2023

Conversation

wikumChamith
Copy link
Member

Setting up servers is problematic when the server file path contains spaces. So this adds an error message for server directories with spaces.

@dkayiwa
Copy link
Member

dkayiwa commented Aug 15, 2023

Should it be only the server? Or anything in the path that has spaces?

@wikumChamith
Copy link
Member Author

Should it be only the server? Or anything in the path that has spaces?

@dkayiwa I think this going to only be problematic with the server path.

@dkayiwa
Copy link
Member

dkayiwa commented Aug 15, 2023

During the conference hackathon, one of the participants had a user account with space hence leading to a path that has space. The server id was fine.

@wikumChamith
Copy link
Member Author

During the conference hackathon, one of the participants had a user account with space hence leading to a path that has space. The server id was fine.

@dkayiwa This code checks the whole server path. So this covers both the user name and server name.

eg: serverDir.getAbsolutePath() = /home/wikum/openmrs/server new

@wikumChamith
Copy link
Member Author

@dkayiwa do you think we should have a separate error message for invalid usernames??

@dkayiwa
Copy link
Member

dkayiwa commented Aug 15, 2023

All we need is communicate to the user about the cause of the failure being any space along the full path from the beginning to the end.

@wikumChamith
Copy link
Member Author

All we need is communicate to the user about the cause of the failure being any space along the full path from the beginning to the end.

@dkayiwa doesn't the current error message convey that??

@dkayiwa dkayiwa merged commit f2367e7 into openmrs:master Aug 16, 2023
2 checks passed
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.

2 participants