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

Remove GormDB from query functions that already used SQL #3097

Merged
merged 4 commits into from
Sep 6, 2022

Conversation

dnephin
Copy link
Contributor

@dnephin dnephin commented Sep 1, 2022

Summary

Branched from #3094

This PR removes GormTxn from a few functions that were already using regular SQL through the gorm interface.

Related Issues

Related to #2415

@dnephin dnephin changed the title Remove GormTxn from query functions that already used SQL Remove GormDB from query functions that already used SQL Sep 1, 2022
Copy link
Collaborator

@BruceMacD BruceMacD left a comment

Choose a reason for hiding this comment

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

Just some suggestions for consistency

internal/server/data/identity.go Outdated Show resolved Hide resolved
internal/server/data/identity.go Outdated Show resolved Hide resolved
internal/server/data/identity.go Outdated Show resolved Hide resolved
internal/server/data/provider.go Outdated Show resolved Hide resolved
@dnephin dnephin force-pushed the dnephin/data-access-key-queries-2 branch from 06225fa to 527b6a1 Compare September 2, 2022 19:19
@dnephin dnephin force-pushed the dnephin/data-replace-gorm-txn-1 branch from 4f4fec2 to 055e9f4 Compare September 2, 2022 19:22
@dnephin dnephin force-pushed the dnephin/data-access-key-queries-2 branch 2 times, most recently from 9136f44 to fcf88ac Compare September 6, 2022 16:03
@dnephin dnephin force-pushed the dnephin/data-replace-gorm-txn-1 branch from 055e9f4 to 63fb1c0 Compare September 6, 2022 16:04
@dnephin dnephin force-pushed the dnephin/data-access-key-queries-2 branch from fcf88ac to a545a90 Compare September 6, 2022 19:53
@dnephin dnephin force-pushed the dnephin/data-replace-gorm-txn-1 branch from 63fb1c0 to b882063 Compare September 6, 2022 22:26
@dnephin dnephin force-pushed the dnephin/data-access-key-queries-2 branch from a545a90 to 333b84c Compare September 6, 2022 22:39
Use the stdlib interface or data interface.
In the future this should return the value of a field on data.DB. For now we can get the reference
from gorm. The panic is added instead of an error because if we can't get the sqlDB this represents a
programming error. We always call this on the data.DB, which should always have the right state.

This commit removes another call to GormTxn.
@dnephin dnephin force-pushed the dnephin/data-replace-gorm-txn-1 branch from b882063 to 63c1085 Compare September 6, 2022 22:41
Co-authored-by: Bruce MacDonald <brucewmacdonald@gmail.com>
Base automatically changed from dnephin/data-access-key-queries-2 to main September 6, 2022 22:51
@dnephin dnephin merged commit 361fbbe into main Sep 6, 2022
@dnephin dnephin deleted the dnephin/data-replace-gorm-txn-1 branch September 6, 2022 23:18
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.

2 participants