woodpecker-ci / woodpecker

Woodpecker is a simple yet powerful CI/CD engine with great extensibility.
https://woodpecker-ci.org
Apache License 2.0
3.89k stars 346 forks source link

Fix helper functions for MySQL syntax #3874

Open lafriks opened 5 days ago

lafriks commented 5 days ago

Fixes #3870

Would be nice if someone could test it as I have no mysql/mariadb instances to reproduce this on

rubenelshof commented 5 days ago

I can help testing it if you want. Please let me know if I have to do something specifically for the test.

codecov[bot] commented 5 days ago

Codecov Report

Attention: Patch coverage is 0% with 34 lines in your changes missing coverage. Please review.

Project coverage is 27.35%. Comparing base (fb37147) to head (d78ea31). Report is 1 commits behind head on main.

Files Patch % Lines
server/store/datastore/migration/common.go 0.00% 34 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3874 +/- ## ========================================== - Coverage 27.37% 27.35% -0.02% ========================================== Files 384 384 Lines 26545 26572 +27 ========================================== + Hits 7267 7270 +3 - Misses 18611 18636 +25 + Partials 667 666 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

6543 commented 5 days ago

I can help testing it if you want. Please let me know if I have to do something specifically for the test.

the pipeline should now publish an image you can test against

rubenelshof commented 5 days ago

I just tested it and it fails with the following error.

8:58AM INF woodpecker/src/github.com/woodpecker-ci/woodpecker/shared/logger/logger.go:101 > log level: debug
8:58AM FTL woodpecker/src/github.com/woodpecker-ci/woodpecker/cmd/server/main.go:46 > error running server error="can't setup store: could not migrate datastore: migration alter-table-registries-fix-required-fields failed: Error 1054 (42S22): Unknown column 'column_name' in 'where clause'"
lafriks commented 5 days ago

as soon as CI builds new image you can try again, should be fixed now

rubenelshof commented 5 days ago

It failed again.

10:14AM INF woodpecker/src/github.com/woodpecker-ci/woodpecker/shared/logger/logger.go:101 > log level: debug
10:14AM FTL woodpecker/src/github.com/woodpecker-ci/woodpecker/cmd/server/main.go:46 > error running server error="can't setup store: could not migrate datastore: migration alter-table-registries-fix-required-fields failed: column repo_id data type in table registries can not be detected"

I dont know if this helps but I executed the query prior to the error I see in the common.go file and get the following results:

MariaDB [woodpecker]> show columns from registries;
+----------+---------------+------+-----+---------+----------------+
| Field    | Type          | Null | Key | Default | Extra          |
+----------+---------------+------+-----+---------+----------------+
| id       | bigint(20)    | NO   | PRI | NULL    | auto_increment |
| repo_id  | bigint(20)    | YES  | MUL | 0       |                |
| address  | varchar(255)  | YES  | MUL | NULL    |                |
| username | varchar(2000) | YES  |     | NULL    |                |
| password | text          | YES  |     | NULL    |                |
+----------+---------------+------+-----+---------+----------------+
5 rows in set (0.002 sec)

MariaDB [woodpecker]> show columns from registries where lower(field) = 'repo_id';
+---------+------------+------+-----+---------+-------+
| Field   | Type       | Null | Key | Default | Extra |
+---------+------------+------+-----+---------+-------+
| repo_id | bigint(20) | YES  | MUL | 0       |       |
+---------+------------+------+-----+---------+-------+
lafriks commented 3 days ago

could be resultset is actually not lowercase as I thought, will try to use Type instead of type

lafriks commented 3 days ago

please test again when new version is built

rubenelshof commented 3 days ago

This works and fixes the issue. No errors reported in the logs.

lafriks commented 3 days ago

can you show output of show columns from registries; after migrations?

rubenelshof commented 3 days ago

can you show output of show columns from registries; after migrations?

+----------+---------------+------+-----+---------+----------------+
| Field    | Type          | Null | Key | Default | Extra          |
+----------+---------------+------+-----+---------+----------------+
| id       | bigint(20)    | NO   | PRI | NULL    | auto_increment |
| repo_id  | bigint(20)    | NO   | MUL | 0       |                |
| address  | varchar(255)  | NO   | MUL | NULL    |                |
| username | varchar(2000) | YES  |     | NULL    |                |
| password | text          | YES  |     | NULL    |                |
| org_id   | bigint(20)    | NO   | MUL | 0       |                |
+----------+---------------+------+-----+---------+----------------+
lafriks commented 3 days ago

Great! Everything looks as it should be. Thanks for your help on testing it! ❤️