MDBF-916 Implementing bb's standard db access pattern for last-N-failed builder

[RazvanLiviuVarzaru]

MTRLogObserver is deprecated starting with Buildbot 3.3.0. Where possible, without breaking existing builders, we need to move away from using MTR related classes and methods.

In this case, subclassing MTR was not even needed for getting the test failures.

This patch is implementing the standard db access pattern of Buildbot. See:
https://buildbot.readthedocs.io/en/latest/developer/database.html#module-buildbot.db.pool

Although the recommended way is to retrieve data through the data api our use case is special enough to make an exception, given that the test_ table schemas will become in-house maintained when we migrate to newer versions of buildbot.

VERY IMPORTANT NOTE:
After removing MTR as a parent class I observed that tests_to_run start to get populated on buildbot.dev.mariadb.org Countless hours later, some debugging code revelead that test_type is sent as 'None' to get_tests_for_type.

2026-02-02 16:11:40+0000 [-] [FetchTestData] master_branch: type=<class 'str'> len=5 repr='10.11'
2026-02-02 16:11:40+0000 [-] [FetchTestData] master_branch tail codepoints: [49, 48, 46, 49, 49]
2026-02-02 16:11:40+0000 [-] [FetchTestData] test_type: type=<class 'NoneType'> len=None repr=None

The only reason that on buildbot.mariadb.org, tests_run_run is sometimes populated is because the failures are bumped with the default type as in:

               tests += yield self.get_tests_for_type(
                    branch, "mtr", limit - len(tests)
                )

The mtr type last appeared on 2025-01-28 as per cross-reference: https://buildbot.mariadb.org/cr/?branch=&revision=&platform=&dt=&bbnum=&typ=MTR&info=&test_name=&test_variant=&info_text=&failure_text=&limit=5#

So basically the last-N-failed builder never did what it was supposed to do. The source of this bug is setting test_type before invoking the __init__ method of the parent class (MTR) in:

class FetchTestData(MTR):
    def __init__(self, mtrDbPool, test_type, **kwargs):
        self.mtrDbPool = mtrDbPool
        self.test_type = test_type
        super().__init__(dbpool=mtrDbPool, **kwargs)

Some will guess right, MTR defines test_type as None (the default).

[RazvanLiviuVarzaru]

@vuvova since you were the original implementer of this class, can I have your review please?

Also, please see the VERY IMPORTANT NOTE section, which basically proves us that the last-N-failed builder hasn’t really done much so far, unfortunately.
So, merging this in production will allow the builder to get the right tests, and I don't know what will happen.

My advice is that you should remove it from branch protection, fix any failing tests that are now being uncovered then back to branch protection when is stable. Otherwise, there's a chance it will block many PR's for old bugs. Your call.

[vuvova]

I can hardly review it, I don't know SQLalchemy, and can barely use twisted

[RazvanLiviuVarzaru]

I can hardly review it, I don't know SQLalchemy, and can barely use twisted

@vuvova Understood, I assigned you based on git history, you were the original author.
In this case, please ACK the note in: #904 (comment), because this patch also fixes a bug in the builder.