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

fix: plumb context into config parameter db methods #464

Merged
merged 5 commits into from Mar 6, 2024
Merged

Conversation

mistahj67
Copy link
Contributor

Description

Plumb context into gorm config parameters db methods
https://gorm.io/docs/context.html#Context-Timeout

Motivation and Context

Currently as it stands any endpoints that rely on purely gorm do not respect request.Context and as such timeouts are not respected either. While we want to eventually get to the removal of the gorm dependency altogether, that is a long way and a lot of effort away and this I think this approach would give us that much room to maneuver while we get there.

How Has This Been Tested?

Using a rest client and the prefer header
Add a sleep inside of the handler
Look for the 500 request timed out after exceeding the timeout. (Note: the request will take as long as the sleep is set, it's not tied to the prefer header)

Types of changes

  • Chore (a change that does not modify the application functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Documentation updates are needed, and have been made accordingly.
  • I have added and/or updated tests to cover my changes.
  • All new and existing tests passed.
  • My changes include a database migration.

Copy link
Contributor

@superlinkx superlinkx left a comment

Choose a reason for hiding this comment

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

Minor nit, but doesn't require a change: whenever possible, set the mock expectation to the correct value (especially with context) rather than gomock.Any() this ensures that the mock actually sees the thing you think you're passing it which adds a bit more validation. It's not the biggest deal, but it's nice to have something more concrete than Any()

@mistahj67 mistahj67 merged commit 6486ff3 into main Mar 6, 2024
3 checks passed
@mistahj67 mistahj67 deleted the BED-4179 branch March 6, 2024 14:39
@github-actions github-actions bot locked and limited conversation to collaborators Mar 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants