-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Added occ install option for database-port #323
Conversation
Extended the database setup to store the database port. Changed the PostgreSQL connection error message for clarification.
@Faldon, thanks for your PR! By analyzing the annotation information on this pull request, we identified @bartv2, @DeepDiver1975 and @nickvergessen to be potential reviewers |
@@ -50,6 +50,7 @@ protected function configure() { | |||
->addOption('database', null, InputOption::VALUE_REQUIRED, 'Supported database type', 'sqlite') | |||
->addOption('database-name', null, InputOption::VALUE_REQUIRED, 'Name of the database') | |||
->addOption('database-host', null, InputOption::VALUE_REQUIRED, 'Hostname of the database', 'localhost') | |||
->addOption('database-port', null, InputOption::VALUE_OPTIONAL, 'Port the database is listening on') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be InputOption::VALUE_REQUIRED
Options are all optional, but when you specify it, it needs a value.
Ofc, casting to int is much more appropriate. And in case the port was provided with the hostname, addslashes is already executed on the string. |
… was provided. Added casting database port to int for input sanitation in pgsql and oci connections.
👍 |
Tested and works 👍 |
Implemented non-standard database port on occ install
Tested
ToDo
Solves #271