Skip to content

codemonitors: Clean up CodeMonitorStore

Warren Giffordrequested to merge
cc/simplify-action-emails into main

Created by: camdencheek

Welcome to this monster housecleaning PR for CodeMonitor store. I started making small housecleaning PRs to bring our CodeMonitorStore up to current standards and make it a more idiomatic store, but there were many changes, so I decided it was better to rip off the band-aid and make one massive PR with all the changes. I could make these commits all separate PRs, but then it would take all week to get them reviewed, rebased, and merged.

A brief tour of what you'll find in this PR:

  • Renaming methods to be more consistent and read more clearly (see Glossary for details)
  • Inlining one-off query creation methods
  • Use QueryRow where applicable for easier scanning/error handling
  • Reuse defined list of columns between queries and scanners to ensure they always agree
  • Make all integer types explicitly agree with the database integer types (integer/serial -> int32 and bigint/bigserial -> int64)
  • Always close *sql.Rows in the function where it was created
  • Add comments
  • Removed named return parameters
  • Remove predeclared variables where not needed
  • Add some comments (probably could still use more, particularly with the info in the glossary below)

Glossary:

  • Trigger: an event that "triggers" a set of code monitor actions. Currently our only trigger is a QueryTrigger.
  • Action: a response to a trigger event. Currently, our only action is EmailAction, but this will be expanded very soon to include Webhook and SlackWebhook.
  • Job: an asynchronously scheduled task that is executed by a worker. In the context of code monitors, this will be either a trigger job (execute a search and see if there are results) or an action job (send an email, execute a webhook, etc.)
  • TriggerJob: an execution of a trigger check to see if there are any new events. In this case, all trigger jobs are search executions.
  • ActionJob: an execution of a configured Action. Currently, only "send an email".
  • QueryTrigger: a Trigger that checks if the results of a commit or diff search returns any new results
  • EmailAction: an Action that sends an email to the configured recipient in response to an event from a Trigger

Review notes:

  • Each commit is small, self-contained, and (I think) self-explanatory, so if you'd like to review this closely, I recommend looking at it commit-by-commit.
  • These changes are purely style changes. This should be no external change to behavior.

What this PR does not cover:

  • Removing dependency on the graphqlbackend package
  • Moving the store into enterprise/internal/database (dependent on the removal of graphqlbackend dependency)
  • Removing dependency on the workerutil package
  • Remove dbconn.Global in tests
  • Use database now() rather than passing it in the query. This might not be reasonable depending on tests.

Merge request reports