Skip to content

use pdo for postgres setup - #381

Merged
rullzer merged 4 commits into
masterfrom
postgres-setup
Jul 15, 2016
Merged

rullzer merged 4 commits into
masterfrom
postgres-setup

Conversation

@icewind1991

Copy link
Copy Markdown
Member

Removes the need of having the pgsql extension installed by using pdo/doctrine during setup

Fixes #367

@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Jul 12, 2016
@icewind1991 icewind1991 added this to the Nextcloud Next milestone Jul 12, 2016
@mention-bot

Copy link
Copy Markdown

@icewind1991, thanks for your PR! By analyzing the annotation information on this pull request, we identified @bartv2, @DeepDiver1975 and @Faldon to be potential reviewers

@MorrisJobke

Copy link
Copy Markdown
Member
There were 2 failures:

1) Test\SetupTest::testGetSupportedDatabasesWithNoWorking
OC\Setup::getAvailableDbDriversForPdo() was not expected to be called more than once.

/drone/src/github.com/nextcloud/server/lib/private/Setup.php:170
/drone/src/github.com/nextcloud/server/tests/lib/SetupTest.php:90

2) Test\SetupTest::testGetSupportedDatabasesWithAllWorking
OC\Setup::getAvailableDbDriversForPdo() was not expected to be called more than once.

/drone/src/github.com/nextcloud/server/lib/private/Setup.php:170
/drone/src/github.com/nextcloud/server/tests/lib/SetupTest.php:114

Comment thread lib/private/AllConfig.php
return $userIDs;
}

public function getSystemConfig() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

😰 why? just use the methods from above?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The connectionfactory wants a systemconfig instance

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Then mark @internal at least please

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's not exposed in the public interface

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

mmmm Maybe we should change the connectionfactory at some point then.

@MorrisJobke MorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jul 12, 2016
@icewind1991

Copy link
Copy Markdown
Member Author

Tests are all green now

@icewind1991 icewind1991 added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 14, 2016
@LukasReschke

Copy link
Copy Markdown
Member

👍

1 similar comment
@rullzer

rullzer commented Jul 15, 2016

Copy link
Copy Markdown
Member

👍

@rullzer
rullzer merged commit 6f9236f into master Jul 15, 2016
@rullzer
rullzer deleted the postgres-setup branch July 15, 2016 19:30
$query = $connection->prepare("ALTER ROLE " . addslashes($this->dbUser) . " CREATEDB WITH PASSWORD " . addslashes($this->dbPassword));
} else {
// create the user
$query = $connection->prepare("CREATE USER " . addslashes($this->dbUser) . " CREATEDB PASSWORD " . addslashes($this->dbPassword));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This broke the tests on my app:

Syntax error: 7 ERROR:  syntax error at or near "zxngvsgscub2i9frs64ac9r43ivk6j"

LINE 1: CREATE USER oc_admin CREATEDB PASSWORD zxngvsgscub2i9frs64ac

The ' are missing, I will add them.

https://travis-ci.org/nextcloud/announcementcenter/jobs/145487372

R0Wi pushed a commit to R0Wi/server that referenced this pull request Nov 22, 2025
…ster command (nextcloud#381)

Do not remove ExApp docker volume by default, deprecate old option
`keep-data`, add new `rm-data` to remove ExApp data.

---------

Signed-off-by: Andrey Borysenko <andrey18106x@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants