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

lxd/db: Don't propagate expected errors #12977

Merged
merged 1 commit into from Feb 28, 2024

Conversation

masnax
Copy link
Contributor

@masnax masnax commented Feb 28, 2024

Fixes this error in the CI tests: https://github.com/canonical/lxd-ci/actions/runs/8062199894/job/22021469170#step:6:3297

This was recently refactored here:

https://github.com/canonical/lxd/pull/12754/files#diff-6ba9a4d3cf752f98f07e5a8ff4ba755b19eb813013b0550931532405b46ba9b9

At the end of this block, we return the err var. Previously this block was wrapped in a transaction which was assigned to an error, but the transaction itself would assign err to nil if we received a sql.ErrNoRows error. When the transaction was removed, the return nil line was removed as well, and so the sql.ErrNoRows error was being propagated to the caller, instead of nil.

This was recently refactored here:

https://github.com/canonical/lxd/pull/12754/files#diff-6ba9a4d3cf752f98f07e5a8ff4ba755b19eb813013b0550931532405b46ba9b9

At the end of this block, we return the `err` var.
Previously this block was wrapped in a transaction which was assigned to
an error, but the transaction itself would assign `err` to `nil` if we
received a `sql.ErrNoRows` error. When the transaction was removed, the
`return nil` line was removed as well, and so the `sql.ErrNoRows` error
was being propagated to the caller, instead of `nil`.

Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
@tomponline tomponline merged commit 31f6f91 into canonical:main Feb 28, 2024
27 checks passed
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.

None yet

2 participants