Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(database): move log logic into separate package #722

Merged
merged 66 commits into from
Feb 24, 2023

Conversation

jbrockopp
Copy link
Contributor

@jbrockopp jbrockopp commented Oct 10, 2022

Based off of #574, #663, #687, #692 and #721

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

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

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

// LogService represents the Vela interface for log
// functions with the supported Database backends.
//
//nolint:revive // ignore name stutter
type LogService interface {
// Log Data Definition Language Functions
//
// https://en.wikipedia.org/wiki/Data_definition_language
// CreateLogIndexes defines a function that creates the indexes for the logs table.
CreateLogIndexes() error
// CreateLogTable defines a function that creates the logs table.
CreateLogTable(string) error
// Log Data Manipulation Language Functions
//
// https://en.wikipedia.org/wiki/Data_manipulation_language
// CountLogs defines a function that gets the count of all logs.
CountLogs() (int64, error)
// CountLogsForBuild defines a function that gets the count of logs by build ID.
CountLogsForBuild(*library.Build) (int64, error)
// CreateLog defines a function that creates a new log.
CreateLog(*library.Log) error
// DeleteLog defines a function that deletes an existing log.
DeleteLog(*library.Log) error
// GetLog defines a function that gets a log by ID.
GetLog(int64) (*library.Log, error)
// GetLogForService defines a function that gets a log by service ID.
GetLogForService(*library.Service) (*library.Log, error)
// GetLogForStep defines a function that gets a log by step ID.
GetLogForStep(*library.Step) (*library.Log, error)
// ListLogs defines a function that gets a list of all logs.
ListLogs() ([]*library.Log, error)
// ListLogsForBuild defines a function that gets a list of logs by build ID.
ListLogsForBuild(*library.Build, int, int) ([]*library.Log, int64, error)
// UpdateLog defines a function that updates an existing log.
UpdateLog(*library.Log) error
}

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

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

This engine contains no raw SQL queries for integrating with the logs 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 self-assigned this Oct 10, 2022
database/log/get_service.go Show resolved Hide resolved
database/log/get_step.go Show resolved Hide resolved
database/log/create.go Show resolved Hide resolved
database/log/update.go Show resolved Hide resolved
database/log/count_build_test.go Outdated Show resolved Hide resolved
database/log/get_service_test.go Outdated Show resolved Hide resolved
database/log/get_step_test.go Outdated Show resolved Hide resolved
database/log/create.go Show resolved Hide resolved
database/log/update.go Show resolved Hide resolved
database/log/create_test.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Oct 10, 2022

Codecov Report

Merging #722 (9ae0b99) into main (9e83ba0) will increase coverage by 0.17%.
The diff coverage is 72.93%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #722      +/-   ##
==========================================
+ Coverage   54.70%   54.88%   +0.17%     
==========================================
  Files         232      244      +12     
  Lines       16392    16490      +98     
==========================================
+ Hits         8968     9050      +82     
- Misses       7024     7041      +17     
+ Partials      400      399       -1     
Impacted Files Coverage Δ
api/log.go 0.00% <0.00%> (ø)
api/stream.go 0.00% <0.00%> (ø)
database/log/log.go 51.72% <51.72%> (ø)
database/log/create.go 66.66% <66.66%> (ø)
database/log/update.go 66.66% <66.66%> (ø)
database/sqlite/sqlite.go 66.10% <66.66%> (+1.40%) ⬆️
database/log/list.go 76.92% <76.92%> (ø)
database/log/list_build.go 80.43% <80.43%> (ø)
database/postgres/postgres.go 67.31% <81.25%> (+2.25%) ⬆️
database/log/delete.go 85.71% <85.71%> (ø)
... and 9 more

database/service.go Outdated Show resolved Hide resolved
@jbrockopp jbrockopp marked this pull request as ready for review February 14, 2023 20:32
@jbrockopp jbrockopp requested a review from a team as a code owner February 14, 2023 20:32
Copy link
Contributor

@plyr4 plyr4 left a comment

Choose a reason for hiding this comment

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

LGTM thank you
I left some small nitpick comments that could be addressed separately, non-blocking

database/log/index.go Show resolved Hide resolved
database/log/get.go Show resolved Hide resolved
database/log/get_step.go Show resolved Hide resolved
database/log/get_service.go Show resolved Hide resolved
database/log/list.go Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants