Skip to content

Conversation

@jbrockopp
Copy link
Contributor

@jbrockopp jbrockopp commented May 27, 2021

Leftover work for go-vela/community#248

Related to go-gorm/gorm#4416

This is a workaround to a problem found in an upstream library.

This updates the go-vela/server/database/postgres client to specifically check for the rows returned from queries:

// check if the query returned a record not found error or no rows were returned
if errors.Is(result.Error, gorm.ErrRecordNotFound) || result.RowsAffected == 0 {
return nil, gorm.ErrRecordNotFound
}

You'll note that we are explicitly checking the RowsAffected field is equal to 0 from the result returned by GORM.

This is required due to what initially appears to be a bug in GORM (See above go-gorm/gorm issue for more details).

GORM no longer returns record not found when a query is ran against the database and no rows are returned.

@jbrockopp jbrockopp added the bug Indicates a bug label May 27, 2021
@jbrockopp jbrockopp self-assigned this May 27, 2021
@codecov
Copy link

codecov bot commented May 27, 2021

Codecov Report

Merging #409 (f1af6ec) into master (425ad72) will increase coverage by 0.34%.
The diff coverage is 88.65%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #409      +/-   ##
==========================================
+ Coverage   84.81%   85.15%   +0.34%     
==========================================
  Files         179      179              
  Lines        5050     5073      +23     
==========================================
+ Hits         4283     4320      +37     
+ Misses        507      500       -7     
+ Partials      260      253       -7     
Impacted Files Coverage Δ
database/postgres/user.go 57.69% <42.85%> (+3.84%) ⬆️
database/postgres/log.go 78.08% <85.71%> (+5.47%) ⬆️
database/postgres/repo.go 76.92% <85.71%> (+5.12%) ⬆️
database/postgres/build.go 93.22% <100.00%> (+7.76%) ⬆️
database/postgres/hook.go 89.47% <100.00%> (+6.14%) ⬆️
database/postgres/secret.go 81.96% <100.00%> (+3.12%) ⬆️
database/postgres/service.go 86.20% <100.00%> (+1.02%) ⬆️
database/postgres/step.go 86.20% <100.00%> (+1.02%) ⬆️
database/postgres/worker.go 89.47% <100.00%> (+1.23%) ⬆️

@jbrockopp jbrockopp marked this pull request as ready for review May 27, 2021 18:22
@jbrockopp jbrockopp requested a review from a team as a code owner May 27, 2021 18:22
Copy link
Contributor

@KellyMerrick KellyMerrick left a comment

Choose a reason for hiding this comment

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

lgtm

@jbrockopp jbrockopp merged commit ba294ee into master May 27, 2021
@jbrockopp jbrockopp deleted the fix/database/postgres/record_not_found branch May 27, 2021 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Indicates a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants