Skip to content

Conversation

@jbrockopp
Copy link
Contributor

@jbrockopp jbrockopp commented Sep 11, 2022

Based off of #574, #663 and #687

This change continues the refactor efforts initially introduced in the above PRs.

This adds a new worker package to the github.com/go-vela/server/database package.

This contains a WorkerService interface declaring all functions necessary for worker based interactions with the database:

// WorkerService represents the Vela interface for worker
// functions with the supported Database backends.
//
//nolint:revive // ignore name stutter
type WorkerService interface {
// Worker Data Definition Language Functions
//
// https://en.wikipedia.org/wiki/Data_definition_language
// CreateWorkerIndexes defines a function that creates the indexes for the workers table.
CreateWorkerIndexes() error
// CreateWorkerTable defines a function that creates the workers table.
CreateWorkerTable(string) error
// Worker Data Manipulation Language Functions
//
// https://en.wikipedia.org/wiki/Data_manipulation_language
// CountWorkers defines a function that gets the count of all workers.
CountWorkers() (int64, error)
// CreateWorker defines a function that creates a new worker.
CreateWorker(*library.Worker) error
// DeleteWorker defines a function that deletes an existing worker.
DeleteWorker(*library.Worker) error
// GetWorker defines a function that gets a worker by ID.
GetWorker(int64) (*library.Worker, error)
// GetWorkerForHostname defines a function that gets a worker by hostname.
GetWorkerForHostname(string) (*library.Worker, error)
// ListWorkers defines a function that gets a list of all workers.
ListWorkers() ([]*library.Worker, error)
// UpdateWorker defines a function that updates an existing worker.
UpdateWorker(*library.Worker) error
}

This package also contains the engine which implements the above service interface:

// engine represents the worker functionality that implements the WorkerService interface.
engine struct {
// engine configuration settings used in worker functions
config *config
// gorm.io/gorm database client used in worker functions
//
// https://pkg.go.dev/gorm.io/gorm#DB
client *gorm.DB
// sirupsen/logrus logger used in worker functions
//
// https://pkg.go.dev/github.com/sirupsen/logrus#Entry
logger *logrus.Entry
}

This engine contains no raw SQL queries for integrating with the workers table.

Instead, we leverage our DB library's (https://gorm.io/) agnostic abstraction for integrating with that table.

jbrockopp and others added 30 commits February 23, 2022 13:45
@jbrockopp jbrockopp marked this pull request as ready for review October 6, 2022 15:58
@jbrockopp jbrockopp requested a review from a team as a code owner October 6, 2022 15:58
Copy link
Member

@cognifloyd cognifloyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that this moves from GetWorker to GetWorkerByHostname which is much better.

It also drops GetWorkerByAddress - should we re-add that?

@jbrockopp
Copy link
Contributor Author

@cognifloyd GetWorkerByAddress was not used anywhere in the codebase 😅

When I discovered this during the refactor, I elected to not include it.

However, if we feel there is a good reason to keep it, I could certainly do so.

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

Labels

enhancement Indicates an improvement to a feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants