Use Docker consistently #3884

greg0ire · 2020-02-28T21:04:35Z

Q	A
Type	improvement
BC Break	no
Fixed issues	not an issue, but #3883 (review)

Summary

When a Docker version of the build is available, use it. Also, remove jobs that test the same version as a docker job.

greg0ire · 2020-02-28T21:04:54Z

No idea if this is actually going to work, let's see…

Spoiler alert

It doesn't

greg0ire · 2020-02-28T21:27:53Z

Ok it works now. Please review.

SenseException

A few minutes ago, I wanted to write something about 7.4snapshot, but your recent commit is handling that. 😁

morozov · 2020-02-28T21:43:33Z

I’m all for this with the exception that it will increase the likelihood of #3478 which is already PITA. The tests against Docker-based MySQL (and perhaps MariaDB) images fail from time to time because, despite the readiness probe passing, all subsequent queries fail with "2006 Server gone away".

Before we go all Docker, I want this issue to be addressed or at least mitigated.

Maybe instead of doing \q, we should execute some real query like SELECT NULL FROM information_schema.something in:

sudo docker exec -i mysql80 bash <<< 'until echo \\q | mysql doctrine_tests > /dev/null 2>&1 ; do sleep 1; done'

greg0ire · 2020-02-28T21:58:30Z

Ok, I'm going to try this: https://stackoverflow.com/questions/25503412/how-do-i-know-when-my-docker-mysql-container-is-up-and-mysql-is-ready-for-taking#comment66272388_29793382

And then … trigger 10 builds?

greg0ire · 2020-02-28T22:03:25Z

Not sure if mysqladmin is available in the image, let's see…

greg0ire · 2020-02-28T22:08:33Z

I think I have something more to do to force waiting for the container to be healthy.

greg0ire · 2020-02-28T23:00:38Z

Done, and it seems to work. @morozov what now? Let's restart the build many times?

morozov · 2020-02-28T23:24:27Z

Let's restart the build many times?

I thought about the same. Maybe 5-10 times to be on the safe side. But before that, since we're generalizing the dependencies, why don't we generalize the install-*.sh scripts for MySQL and MariaDB?

Instead of copy/pasting the code, we could introduce a required CLI argument that would take a Docker image name to be spun up like: docker-run-mysql-or-mariadb.sh mysql:8.0.

greg0ire · 2020-02-29T10:21:24Z

I'm on it. I'm going to use an environment variable to specify the image, that way you can still see what the version is in the overview of the jobs.

greg0ire · 2020-02-29T10:56:35Z

Not 100% sure but I think I just made these lines useless:

dbal/.travis.yml

Lines 16 to 17 in 96d10bd

    
           before_script: 
        
             - if [[ "$DB" == "mysql" || "$DB" == "mysqli" || "$DB" == *"mariadb"* ]]; then mysql < tests/travis/create-mysql-schema.sql; fi;

I think they are not in use for (some?) jobs that end with .docker, correct? Why is that?

greg0ire · 2020-02-29T11:03:17Z

Ah that's correct, I thought global and local before_script were merged, they aren't, it's an override.

greg0ire · 2020-02-29T15:23:31Z

Green build again :)

Do you think I should get rid of the .docker part in DB env variables and xml filenames?

greg0ire · 2020-02-29T15:30:27Z

There is an infinite loop now, many containers are unhealthy…

greg0ire · 2020-02-29T15:36:08Z

I added a check that will fail the build on unhealthy container. I noticed docker is run with sudo, why is that?

greg0ire · 2020-02-29T15:43:30Z

Ok so it's unhealthy because I badly implemented the addition of the extra command option, no weird transient issue :)

Hopefully it will work more reliably.

It seems to be enabled by default.

greg0ire · 2020-02-29T19:12:43Z

🤔 no Scrutinizer job now…

morozov · 2020-02-29T19:14:39Z

I'm really not for dropping things as soon as possible, but supporting platforms that are not supported by the vendor themselves seems to be another extreme.

I agree and I'd rather drop their support in a minor release than be puristic about semver and keep dragging this legacy. Although, this PR doesn't drop support for legacy platforms but instead stops testing still supported code.

It seems mostly related to IBM DB2… I did touch that job to remove the services: [docker] section, but can't see how that would explain this drop in coverage.

It may be a Scrutinizer bug (because it's buggy as hell) but maybe not. If we don't see this issue on other PRs, then most likely this one is causing the trouble.

greg0ire · 2020-02-29T19:16:11Z

It may be a Scrutinizer bug (because it's buggy as hell) but maybe not. If we don't see this issue on other PRs, then most likely this one is causing the trouble.

I found a setting about tracking branches, and added 2.10.x as a tracked branch. let's trigger a new build.

greg0ire · 2020-02-29T19:23:45Z

Although, this PR doesn't drop support for legacy platforms but instead stops testing still supported code.

This was done in #3883 actually. How do we usually drop support for legacy platforms?

morozov · 2020-02-29T19:40:20Z

How do we usually drop support for legacy platforms?

AFAIK, we only do that in the next major as required by semver. Doing it earlier will require at least the team’s approval. Apart from that, the community is quite serious about maintaining the BC promise even if there's no practical sense.

greg0ire · 2020-02-29T19:46:28Z

Ok so this time, no coverage drop. Since 2.10.x had no Scrutinizer job (because of settings), there might have been an unnoticed coverage drop there. 😓

greg0ire · 2020-02-29T19:48:25Z

Apart from that, the community is quite serious

This tweet is unavailable

greg0ire · 2020-02-29T19:53:36Z

BTW, apparently, it's possible to specify distro on the per-job basis in Scrutinizer.

Did you mean Travis?

greg0ire · 2020-02-29T19:57:07Z

Anyway, there's probably no need to change distros if we use Docker, right?

EDIT: Ah but that would mean using Docker for Postgres…

They are no longer supported by their vendors, but until we do support them, we should continue testing them.

morozov · 2020-02-29T20:45:02Z

This tweet is unavailable

Weird. The tweet is available (Dec 29, 2019) but the link seems to work only if clicked from the twitter feed, not from another website.

Did you mean Travis?

Yes, Travis.

Ah but that would mean using Docker for Postgres…

Yes. We can do it later if needed.

greg0ire · 2020-02-29T20:49:20Z

@SenseException well that was harder than expected :P

SenseException · 2020-02-29T21:02:01Z

Seems like it.

morozov · 2020-02-29T21:39:37Z

[…] well that was harder than expected :P

You shouldn't have requested my review 😄

@greg0ire

Release [2.10.2](https://github.com/doctrine/dbal/milestone/75) 2.10.2 ====== - Total issues resolved: **4** - Total pull requests resolved: **19** - Total contributors: **10** Improvement,Static Analysis --------------------------- - [3964: Mark every exception as immutable](doctrine#3964) thanks to @greg0ire CI,Improvement,Static Analysis ------------------------------ - [3961: Stop relying on the master version of Psalm](doctrine#3961) thanks to @greg0ire - [3951: Setup static analysis with Psalm](doctrine#3951) thanks to @greg0ire - [3799: Upgrade to PHPStan v0.12](doctrine#3799) thanks to @lcobucci Improvement,Logging,Test Suite ------------------------------ - [3957: Reworked LoggingTest to be able to test Statement::executeUpdate()](doctrine#3957) thanks to @morozov CI,Code Style,Improvement,Strict Typing --------------------------------------- - [3955: Remove baseline](doctrine#3955) thanks to @greg0ire Bug,SQLite,Schema Introspection,Schema Managers ----------------------------------------------- - [3937: Column comment incorrectly introspected on SQLite](doctrine#3937) thanks to @morozov Bug,Documentation,Prepared Statements,Query ------------------------------------------- - [3896: Updated documentation for QueryBuilder::execute() return value type](doctrine#3896) thanks to @morozov Bug,Prepared Statements ----------------------- - [3894: Make sure that the $types array has the same keys $params](doctrine#3894) thanks to @morozov - [3893: Ensure the constructor arguments are passed to custom classes](doctrine#3893) thanks to @duncan3dc - [3843: Fix unquoted stmt fragments backslash escaping](doctrine#3843) thanks to @morozov Documentation,Improvement ------------------------- - [3886: Update readme](doctrine#3886) thanks to @greg0ire - [3834: Fix docblock typos in DriverManager docs](doctrine#3834) thanks to @CHItA CI,Improvement,MariaDB,MySQL ---------------------------- - [3884: Use Docker consistently](doctrine#3884) thanks to @greg0ire - [3478: Improve readiness probe stability for containerized databases on CI](doctrine#3478) thanks to @morozov - [3883: Fix broken build](doctrine#3883) thanks to @greg0ire Bug,Documentation,Query,Query Limit/Offset Modification ------------------------------------------------------- - [3842: Fixed the QueryBuilder::setMaxResults() signature to accept NULL](doctrine#3842) thanks to @morozov Bug,Query --------- - [3832: Fix JOIN with no condition bug](doctrine#3832) thanks to @BenMorel Bug,PostgreSQL,Schema Introspection ----------------------------------- - [3821: &doctrine#91;pg&doctrine#93; fix getting table information if search&doctrine#95;path contains escaped schema name](doctrine#3821) thanks to @linniksa Documentation,Improvement,Logging --------------------------------- - [3812: Fix DebugStack#queries docblock type](doctrine#3812) thanks to @ostrolucky Bug,Regression,Schema --------------------- - [3790: fixed unqualified table name of fk constraints when using schemas](doctrine#3790) thanks to @stlrnz and @Alarich # gpg: Signature made Mon Apr 20 19:59:36 2020 # gpg: using DSA key 2C3A645671828132 # gpg: Can't check signature: public key not found # Conflicts: # README.md

greg0ire force-pushed the use-docker-for-all-rdbms branch from f46dc27 to 7fb5938 Compare February 28, 2020 21:15

greg0ire requested a review from a team February 28, 2020 21:28

SenseException previously approved these changes Feb 28, 2020

View reviewed changes

greg0ire force-pushed the use-docker-for-all-rdbms branch 4 times, most recently from 8f5b3b4 to b18ec2a Compare February 28, 2020 22:43

greg0ire force-pushed the use-docker-for-all-rdbms branch from 073e994 to e029896 Compare February 29, 2020 10:52

greg0ire force-pushed the use-docker-for-all-rdbms branch 4 times, most recently from 04dbe80 to e39698d Compare February 29, 2020 15:10

greg0ire force-pushed the use-docker-for-all-rdbms branch from e39698d to 37ec53c Compare February 29, 2020 15:35

greg0ire force-pushed the use-docker-for-all-rdbms branch 2 times, most recently from 8f00e8b to 1215beb Compare February 29, 2020 16:21

greg0ire added 4 commits February 29, 2020 20:05

Replace external health check with native one

765432a

Hopefully it will work more reliably.

Refactor MySQL and MariaDB run in a single script

e7aef4e

Do not enable docker explicitely

6e5fbcb

It seems to be enabled by default.

Synchronize the number of code coverage runs

398f23e

greg0ire force-pushed the use-docker-for-all-rdbms branch from d9612c3 to 398f23e Compare February 29, 2020 19:05

greg0ire closed this Feb 29, 2020

greg0ire reopened this Feb 29, 2020

Continue testing unsupported versions

6e505de

They are no longer supported by their vendors, but until we do support them, we should continue testing them.

greg0ire force-pushed the use-docker-for-all-rdbms branch from d1aaf21 to 6e505de Compare February 29, 2020 20:21

morozov approved these changes Feb 29, 2020

View reviewed changes

greg0ire merged commit f089a93 into doctrine:2.10.x Feb 29, 2020

greg0ire deleted the use-docker-for-all-rdbms branch February 29, 2020 20:49

morozov mentioned this pull request Mar 16, 2020

Improve readiness probe stability for containerized databases on CI #3478

Closed

morozov assigned greg0ire Mar 16, 2020

doctrinebot mentioned this pull request Apr 20, 2020

Merge release 2.10.2 into 2.11.x #3968

Merged

github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Docker consistently #3884

Use Docker consistently #3884

greg0ire commented Feb 28, 2020 •

edited

Loading

greg0ire commented Feb 28, 2020 •

edited

Loading

greg0ire commented Feb 28, 2020

SenseException left a comment

morozov commented Feb 28, 2020 •

edited

Loading

greg0ire commented Feb 28, 2020

greg0ire commented Feb 28, 2020

greg0ire commented Feb 28, 2020

greg0ire commented Feb 28, 2020

morozov commented Feb 28, 2020

greg0ire commented Feb 29, 2020

greg0ire commented Feb 29, 2020 •

edited

Loading

greg0ire commented Feb 29, 2020

greg0ire commented Feb 29, 2020

greg0ire commented Feb 29, 2020

greg0ire commented Feb 29, 2020

greg0ire commented Feb 29, 2020 •

edited

Loading

greg0ire commented Feb 29, 2020

morozov commented Feb 29, 2020

greg0ire commented Feb 29, 2020

greg0ire commented Feb 29, 2020

morozov commented Feb 29, 2020

greg0ire commented Feb 29, 2020 •

edited

Loading

greg0ire commented Feb 29, 2020

greg0ire commented Feb 29, 2020

greg0ire commented Feb 29, 2020 •

edited

Loading

morozov commented Feb 29, 2020

greg0ire commented Feb 29, 2020

SenseException commented Feb 29, 2020

morozov commented Feb 29, 2020

Use Docker consistently #3884

Use Docker consistently #3884

Conversation

greg0ire commented Feb 28, 2020 • edited Loading

Summary

greg0ire commented Feb 28, 2020 • edited Loading

greg0ire commented Feb 28, 2020

SenseException left a comment

Choose a reason for hiding this comment

morozov commented Feb 28, 2020 • edited Loading

greg0ire commented Feb 28, 2020

greg0ire commented Feb 28, 2020

greg0ire commented Feb 28, 2020

greg0ire commented Feb 28, 2020

morozov commented Feb 28, 2020

greg0ire commented Feb 29, 2020

greg0ire commented Feb 29, 2020 • edited Loading

greg0ire commented Feb 29, 2020

greg0ire commented Feb 29, 2020

greg0ire commented Feb 29, 2020

greg0ire commented Feb 29, 2020

greg0ire commented Feb 29, 2020 • edited Loading

greg0ire commented Feb 29, 2020

morozov commented Feb 29, 2020

greg0ire commented Feb 29, 2020

greg0ire commented Feb 29, 2020

morozov commented Feb 29, 2020

greg0ire commented Feb 29, 2020 • edited Loading

greg0ire commented Feb 29, 2020

greg0ire commented Feb 29, 2020

greg0ire commented Feb 29, 2020 • edited Loading

morozov commented Feb 29, 2020

greg0ire commented Feb 29, 2020

SenseException commented Feb 29, 2020

morozov commented Feb 29, 2020

greg0ire commented Feb 28, 2020 •

edited

Loading

greg0ire commented Feb 28, 2020 •

edited

Loading

morozov commented Feb 28, 2020 •

edited

Loading

greg0ire commented Feb 29, 2020 •

edited

Loading

greg0ire commented Feb 29, 2020 •

edited

Loading

greg0ire commented Feb 29, 2020 •

edited

Loading

greg0ire commented Feb 29, 2020 •

edited

Loading