Skip to content

Conversation

@Al2Klimov
Copy link
Member

just to be sure we're compatible with this RDBMS and not only MySQL.

@Al2Klimov Al2Klimov force-pushed the Al2Klimov-patch-2 branch from 9a88abc to 55f0e41 Compare April 5, 2024 09:50
@cla-bot cla-bot bot added the cla/signed label Apr 5, 2024
@oxzi
Copy link
Member

oxzi commented Jun 17, 2025

While this PR was somewhat forgotten, I just thought about it in spite of #947. How about testing both the latest version as well as the minimum required version for each DBMS?

@Al2Klimov
Copy link
Member Author

They're either not available or don't work and it's a bit out of scope here.

@Al2Klimov Al2Klimov requested a review from oxzi August 20, 2025 11:44
@julianbrost
Copy link
Member

They're either not available or don't work and it's a bit out of scope here.

I wouldn't understand that comment if I didn't happen to look at this PR and it's pipelines at just the right moment by pure chance, i.e. at this run: https://github.com/Icinga/icingadb/actions/runs/17097225377

One job failed because there seems to be no mariadb:10.2.2 image on Docker Hub:

panic: Error response from daemon: manifest for mariadb:10.2.2 not found: manifest unknown: manifest unknown

The other one failed with a MySQL error:

panic: Error 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'ON . TO u1 */' at line 1

This also needs more context: that's an error from the database setup done by icinga-testing.

@julianbrost
Copy link
Member

Fixing that is trivial actually (if you know where to look), so I just created a PR for it: Icinga/icinga-testing#32

@Al2Klimov Al2Klimov force-pushed the Al2Klimov-patch-2 branch 3 times, most recently from 39f2210 to 36f9068 Compare September 15, 2025 12:34
@oxzi
Copy link
Member

oxzi commented Jan 9, 2026

Again, what is the state of this PR, @Al2Klimov? Do you want to proceed here? It was quite hard for me to follow the conversion, especially with the time gap in between.

@Al2Klimov
Copy link
Member Author

Well, you asked #729 (comment) for also including the minimum required RDBMS versions, but this seemed not to work. I've rebased and hope for the best...

@oxzi oxzi added the area/tests label Jan 9, 2026
@lippserd
Copy link
Member

Let's be honest, this is more or less a waste of resources. I would close the PR.

@Al2Klimov
Copy link
Member Author

But all checks pass now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants