Further plans on labeller

[Fildrance]

Proposed activities:

Cleaner migrations

Make adding migrations simple, add public constants for table names, add db configuration into README, add tests that will include db stuff.

• Add FluentMigrator for cleaner multi-sql-server migrations and less custom code for them
• Ditch NAOT as a side-effect of a choice. We most likely would not need any of its benefits and maintainability of NAOT app is generally higher. We cannot spare resources for no gains on side-projects, as i see.
• retarget code for PostgreSQL if possible, before we end up with a lot of data migrations to do
• add DB related integration tests (including CI)
• split debugging-in-container docker-compose (exists) and env compose (todo). Latter one should set up postgres db

Merge with maintainer bot (needs voice chat meeting with at minimum julain and simyon)

• convert to one codebase and standarts,
• migrate existing database
• document implemented features

Add reports about stale PRs

When PR have been marked with 'Awaiting changes' and had not received any changes (new commits) for more then 1 month, it should be marked with 'stale'. Stale PRs should be reported to discord every (week/2weeks/month) so maintainers would either mark them for later rework or just close.

• Add migration for PR cache in app DB
• Add methods for going through PRs in GH API (with paging)
• Options class for 'what do we consider stale'
• Add scheduler job that will sync-up list of PRs and critical stuff (comments and labels)
• Add scheduler job that will check stale PRs and do notifications in some discord channel (Table with checkmarks for notifications sent?). Exclude PRs that have special label (SilenceStaleNotifications or something).
• Add code to handler will modify PRs cache in DB on webhook events (ENSURE THERE WONT BE OVERLAPPING AND DIRTY WRITE WITH SCHEDULED SYNC)

Add reports about stale Discourse treads

When topic in discourse contains tag 'requires solution' and does not have tag 'closed' and have no new posts for 1 week, it should be reported to discord every (2 days/week/2week) so maintainers could ask certain other users to join discussion to resolve problem and condense some solution

• pretty much same stuff that PRs neeed

Pinging in discourse should ping in discord

Discourse should have ability to ping maintainer/group DM of maintainers in discord when they are pinged in topic. Probably would need hook for discourse add/edit post, scan every post for matches.

• Add mapping for GH - KeyCloak - Discord users
• As we will keep mapping only for maintainers (maybe triagers?) this will be limited but thats mostly all what we need.
• probably separate microservice that will have access to modifying keycloak, and provide mapping as user claims?
• pinging in discord should be disablable thing (where?)

ACTUALLY useful health-check, metrics and alerts

• Health-check that will provide DB status
• Heaht-check for github api availability (testing token)
• health-check for discord api availability (testing token and permissions? post and delete message?)
• health-check for discourse api availability (testing token? how to check for permissions?)
• metrics:
  • attach metrics reporting
  • github event processed counter
  • github event failed processed counter (exceptions in main method)
  • etc

[Fildrance]

Questions to discuss in context of merging with maintainer bot there are several questions:

Do we keep Dapper or we do EF?

Dapper cons

• Dapper is error-prone, and generally requires MORE integration tests to just prove that after initial development or even some minor refactoring nothing was broken in script or how parameters are passed.
• Dapper is also generally pretty darn stupid with fields mapping (but everyhting can be fixed if you smart enough and able to put some boilerplate)
• Dapper requires knowledge of plain sql which can make some possible contributors a bit scared of touching
• It is VERY vulnurable to refactoring related errors - sql text is not related to code in any way

Dapper pros

• Dapper goes well with NAOT
• Dapper gives more direct control upon what will be called
• Its more performant in general

EF cons

• Even simple scenarios require basic knowledge of LINQ
• DbContext requires boilerplate
• Configuring mapping could be viewed as clumsy and kinda error prone. If you reuse constants from migrations it will become cumbersome.
• Requires proper scoping and disposing
• Easy to misuse (put DbContext related calls in the middle of business code, instead of calling repository)
• Hard to balance re-using DbContext object with not leaking abstration of repository
• Problems with mass insert (more then severl hundreds require turning Tracking off, more then several thousands require DbContext re-creation)
• Complex scenarios, such as 'do not insert row on conflict in uniq index' are quite a bit more painful to implement.
• to ensure configuration is correct it is good practice to have unit tests with logger outputting what EF actually executes - this is kind of boilerplate, but it would enable to destinguish poorly optimised code instantly, and not after perf degradation

EF pros

• Makes Db related code refactorable
• LINQ provides much simpler way to query data. MUCH MUCH simpler.
• Entity classes wont require additional boilerplate classes (MOST OF THE TIME) for Insert/Query in comparison to Dapper
• Have a lot of integrated features, and even more from third-party libraries. For example snake_case convention for fields naming in case we use postgresql (it works poorly with PascalCase, better use snake_case) or logging. Adding things like unified action audit is much easier while having pre-definied seams like interceptors.

Overall i'd say that on a scale of 0 to 10 Dapper gets 3 points on maintainability, but 7 in attracting new contributors (as SQL is more widely known by broader audience, non c# coders) and EF gets 5 points on maintainability but 5 in attracting new contributors. Complex software such as ORM or DB driver are always a downgrade maintainability-wise, but they are what we can tolerate to get stability gain from usage of databases.

EITHER WAY i assume retaining Repository classes to contain DB related code is a must have. And we should not in any case use EF migrations - they have VERY poor readability and are cumbersome.

FastEndpoints VS home brew dispatcher

FastEndpoints is library that looks like bastard child of MediatR and MinimalApi. I had experience with building one of such things myself and i have to say - it being kinda sewed up into http api is a bit silly :( But i see that project have actually very good support in community (including commercial) so i guess its fine? I think it will be a little bit of a maintainability problem but it will add flexibility out of the box so it sounds like its worth investment.

• I am a bit afraid about custom model binding for github events as its a bit complex (need to get part of data from header, then deserialize body using it) but it looks like its handlable.
• Having Validators as built in feature is a big plus.
• Custom gh api authorization is another a bit scary thing, i am not 100% sure it will be easy there.

Job scheduler

That one is pretty much solved, but i am not sure on the order - do we migrate to single project first, and then change job scheduler library, or do we change it first?...

External systems data

To act on background events (initiated not directly by call to our system, like topic in discourse getting stale for example) we would need to set up data storage for external objects. It seems reasonable to support 2 ways of accumulating data this way - both scheduled sync of all records in local and external storage and upserting on events from webhooks.
But to not make it painful later on i'd propose to try and get all the data that could be reasonably useful into local caching database right away, and not try to put as little data as possible - otherwise migrations will eat up a lot of our resources later. I am talking not about 'get all the data that GH exposes' but more of 'try to think what could be useful in future and store it right now'.

[Fildrance]

I do not like the code that adds topic to DB, it is not correct thing to do w/o transaction, but opposite options is not something pretty too... kinda.
Also we need retry policy for HttpClient calls (all of them, i suppose) with correct handling of 429 (too many requests) status code. (fixed in #9)

[juliangiebel]

NuGet packages we decided on

• https://github.com/Arcenox-co/TickerQ
• https://github.com/Cysharp/MessagePipe (Used in SS14.Jetfish)

Combining the maintainer bot and SS14.Labeller

We should reimplement the business logic from the command and event handlers the maintainer bot has into SS14.Labeler.
Basically it just needs to cover the same workflows and the Labellers architecture is a bit to different to just combine all of the maintainer bot with it

[juliangiebel]

Making the Labeller a proper GitHub app

The GitHub http client should be changed to use GithubApp authentication instead of just a token. We can just set the repoId in the configuration and the Labeller shouldn't be openly installable.

[Fildrance]

Need to look at ss14.Auth for how metrics are exposed