catalyst / catalyst-moodle-workflows

4 stars 9 forks source link

Add options to the matrix allowing to specify database versions. #56

Closed katerynadegtyariova closed 2 years ago

katerynadegtyariova commented 2 years ago

This change will allow using a recommended version of the database depending on the requirements in each particular moodle branch.

This is mostly related to Moodle master (ver 401 at this time) requirement to use Postgres 12 or newer.

Each row in the matrix must include the version for both mariadb and postgresql. Although each test only uses one DB service, both DB services will run and for this reason the unneeded DB version could not be skipped.

There was an idea to provide only the version for the DB, which will be tested in the particular test. However, Git action syntax does not allow certain things like using conditions to generate the default version for the database, which was not specified in the matrix.

keevan commented 2 years ago

Running test here since no examples were provided for this PR.

https://github.com/catalyst/moodle-tool_dataflows/pull/344

keevan commented 2 years ago

I'm happy to get this over the line so the tests can resume as normal, but ideally it would use a higher version of:

Allowing this to be dynamic would be a nice to have, and might reduce the need to update this periodically. But that can be pushed down the road if we can get this working again relatively easily.

I'll split this into another enhancement issue.

keevan commented 2 years ago

@katerynadegtyariova can you please take a look at why it's failing when I try to test it the dataflows repo?

https://github.com/catalyst/moodle-tool_dataflows/actions/runs/2736383526

The image (version) seems like it has not resolved correctly.

katerynadegtyariova commented 2 years ago

The test run is failing because it is partially using CI from the main branch instead of the feature branch:

https://github.com/catalyst/catalyst-moodle-workflows/blob/main/.github/workflows/ci.yml#L93

Unfortunately, I cannot point it to add-db-ver-main because after merging it will use the wrong branch.

katerynadegtyariova commented 2 years ago

The successful run is here:

https://github.com/catalyst/moodle-tool_cloudmetrics/actions/runs/2702486444

it was specifically modified to use add-db-ver-main everywhere instead of main

But for the PR it had to be changed back.

katerynadegtyariova commented 2 years ago

pushed the changes to the branch names into add-db-ver-main-test.

https://github.com/catalyst/catalyst-moodle-workflows/compare/add-db-ver-main-test..add-db-ver-main

The test here is passing now, no failures:

https://github.com/catalyst/moodle-tool_dataflows/pull/344