- First commit to issue fork.
- Open on Drupal.org βEnvironment: PHP 8.2 & MySQL 8last update
over 1 year ago Not currently mergeable. - @fjgarlin opened merge request.
- last update
over 1 year ago 30,056 pass - Status changed to Needs review
over 1 year ago 8:19am 23 August 2023 - πͺπΈSpain fjgarlin
I ran into the same issue while converting DrupalCI to GitlabCI and testing PostgreSQL.
The new MR for 11.x is https://git.drupalcode.org/project/drupal/-/merge_requests/4627
See the before and after for "PhpUnitCliTest":
* Before (error): https://git.drupalcode.org/project/gitlab_ci_testbed_for_drupal_core/-/j...
* After (fixed): https://git.drupalcode.org/project/gitlab_ci_testbed_for_drupal_core/-/j... - Status changed to Needs work
over 1 year ago 4:58pm 27 August 2023 - πΈπ°Slovakia poker10
Thanks for working on the MR @fjgarlin!
I think the issue summary needs to be updated, because some parts were fixed in other issues in the meantime (for example the broken exec call was fixed here: #2259947: Minor bug fixes in database system β ). Therefore this issue should only focus on detecting a locale (if possible).
The points raised in #15 and #19 are good, and if we are making changes, I think it will be the best to detect, if the database is run locally. If yes, we can use the approach proposed in the MR currently (try to detect locale and if the creation fail, then fallback to
en_US
). But if the database is not run locally, I think there is no point doing that "blind" attempt with locale from different system, as the separate database server could be entirely different. I propose that in case the database is not run locally, we skip the locale detection and try the luck withen_US
only.What do you think?
- πͺπΈSpain fjgarlin
Updated issue summary to reflect the part that needs fixing. Thanks for the suggestion.
Re detecting if the database runs locally, the DB server could use an alias that "looks" and still be local. And in any case, I think the approach shouldn't change (see below paragraph).
I think that the same approach would work well for both local and remote DBs. We should try to detect the web server locale and then try to set that value for the database (whether it's local or remote), and if it fails then fallback to "en_US" (whether it's local or remote). I know that if the DB is remote we'll be trying blindly, but why not try with the desired value first, and otherwise with the fallback (which will actually be a guess as well).
I think the MR can be simplified a little, so I'll iterate over it.
- last update
over 1 year ago 30,060 pass - Status changed to Needs review
over 1 year ago 7:44am 28 August 2023 - πΈπ°Slovakia poker10
Thanks @fjgarlin for updating the IS, MR and your explanation! I think that this is a good point:
the DB server could use an alias that "looks" and still be local.
Then I think that it could probably stay as it is, without complicating things. Let's keep it in NR to get more reviews.
- π¬π§United Kingdom catch
This is blocking gitlab CI. Could use review from the postgres maintainers.
- last update
over 1 year ago 30,136 pass - last update
over 1 year ago 30,136 pass - last update
over 1 year ago 30,136 pass 41:07 40:09 Running- Status changed to RTBC
over 1 year ago 12:24pm 4 September 2023 - π³π±Netherlands daffie
The MR looks good to me.
I am not totally sure that we cannot create a PostgreSQL database with ctype and collate that will not work with Drupal. That said, we do not get any complains on the issue queue about it. Therefor for me it is not a big problem. When we get complains about this we create a better fix + testing. For now, lets fix Gitlab CI. That is far more important!
The testbot is green.
For me it is RTBC. - Status changed to Fixed
over 1 year ago 12:30pm 4 September 2023 - π¬π§United Kingdom catch
Committed/pushed to 11.x and cherry-picked to 10.1.x, thanks!
Automatically closed - issue fixed for 2 weeks with no activity.