-
Notifications
You must be signed in to change notification settings - Fork 42
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
Pass locale environment to initdb and pg_ctl #334
Conversation
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.
Somehow it seems we've forgot about the initdb, or we skipped
src/pytest_postgresql/executor.py
Outdated
@@ -137,7 +137,7 @@ def init_directory(self): | |||
# remove old one if exists first. | |||
self.clean_directory() | |||
init_directory = [self.executable, 'initdb', '--pgdata', self.datadir] | |||
options = ['--username=%s' % self.user] | |||
options = ['--locale=C', '--username=%s' % self.user] |
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.
Could you check if it would be enough to pass env=self._envvars to both check_output calls wouldn't do the trick as well.
Wouldn't want to have to check it in more than one palces, or have two equal, but different implementations if that's not really required.
Could you check if it would be enough to pass env=self._envvars to both check_output calls wouldn't do the trick as well.
Yes, that works too. I'll update the PR.
Thanks for the review!
|
src/pytest_postgresql/executor.py
Outdated
@@ -138,6 +138,7 @@ def init_directory(self): | |||
self.clean_directory() | |||
init_directory = [self.executable, 'initdb', '--pgdata', self.datadir] | |||
options = ['--username=%s' % self.user] | |||
env = self._envvars.copy() |
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.
I'm not sure why the osx build fails now for the innitdb, but won't fail for the start command itself...
Maybe we're missing some more environment variables now. In a base class it's being done like that:
https://github.com/ClearcodeHQ/mirakuru/blob/master/src/mirakuru/base.py#L216
So we copy current envvars and update it with self._envvars
. Alternative would be to use self._popen_kwargs()['env']
here.
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.
This is not better, see ff31a3f. The actual problem on osx is:
initdb: error: invalid locale settings; check LANG and LC_* environment variables
pg_ctl: database system initialization failed
(e.g. at https://travis-ci.org/github/ClearcodeHQ/pytest-postgresql/jobs/731374804#L414)
So it seems that C.UTF-8
is not available on travis' osx.
We now pass the environment dict containing locale settings to calls of 'initdb' and 'pg_ctl'. This is so as to avoid initdb to pick the locale setting of its execution environment [1] which, when not an English one, will then make the checks in PostgreSQLExecutor.running() (where the message "pg_ctl: server is running" is expected) fail. Fix ClearcodeHQ#315. [1]: https://www.postgresql.org/docs/current/locale.html
I think I'll merge that and try to fix it after. thanks @dlax ! |
Fixes #315.
We pass the environment dict containing locale settings to calls of 'initdb' and 'pg_ctl'. This is so as to avoid initdb to pick the locale setting of its execution environment which, when not an English one, will then make the checks in PostgreSQLExecutor.running() (where the message "pg_ctl: server is running" is expected) fail.