fix: failing queries (GET /accounts?expand=volumes) when migrations are not terminated - #723
Conversation
…re not terminated
WalkthroughThe pull request updates several configuration and migration files. In the Changes
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #723 +/- ##
==========================================
- Coverage 81.97% 81.82% -0.16%
==========================================
Files 135 135
Lines 7291 7291
==========================================
- Hits 5977 5966 -11
- Misses 1008 1016 +8
- Partials 306 309 +3 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
internal/storage/bucket/migrations/11-make-stateless/up.sql (2)
580-601: Consider adding volatility qualifier to get_all_assets functionThe function is properly implemented with the search path fix, but unlike other functions, it doesn't have an explicit volatility qualifier (like
stableorimmutable). Since this function doesn't modify the database and would return the same results for the same parameters, consider adding astablequalifier.create or replace function get_all_assets(_ledger varchar) returns setof varchar + stable language sql as
603-612: Consider adding parallel safe attribute for consistencyThe function
get_account_aggregated_volumeshas theparallel safeattribute, but this one doesn't. If this function can also be safely executed in parallel, consider adding this attribute for consistency and potential performance benefits.create or replace function get_account_aggregated_effective_volumes(_ledger varchar, _account_address varchar, _before timestamp default null) returns jsonb language sql stable + parallel safe as
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.gitignore(1 hunks)Earthfile(1 hunks)internal/storage/bucket/migrations/11-make-stateless/up.sql(2 hunks)tools/generator/Earthfile(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tools/generator/Earthfile
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Tests
🔇 Additional comments (5)
.gitignore (1)
10-11: LGTM: Version control configuration updates are appropriateThe changes to the
.gitignorefile look good:
- Retaining
!.envrcensures that this environment configuration file remains tracked in the repository- Adding
Pulumi.*.yamlto ignore Pulumi configuration files is appropriate as these often contain environment-specific settingsEarthfile (1)
4-4: Dependency update from v0.19.0 to v0.19.1 is appropriateThis is a minor version update of the core dependency, which follows semantic versioning principles and should be backward compatible.
internal/storage/bucket/migrations/11-make-stateless/up.sql (3)
562-564: Good documentation for the search path fixThe comments clearly explain the purpose of the changes - recreating functions with proper search path configuration for databases created before version 2.2.
565-578: Function definition looks good with search path fixThe
get_transactionfunction is properly defined with theset search_path from current;statement to fix the search path issue on older databases.
614-623: Function definition with parallel safe attribute looks goodThe
get_account_aggregated_volumesfunction is properly defined with bothstableandparallel safeattributes, along with the search path fix.
2 participants
No description provided.