-
Notifications
You must be signed in to change notification settings - Fork 349
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
chore: don't use a seperate schema for views_and_triggers #9392
Conversation
✅ Deploy Preview for determined-ui canceled.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9392 +/- ##
==========================================
- Coverage 46.03% 46.01% -0.02%
==========================================
Files 1228 1228
Lines 155889 155874 -15
Branches 2440 2440
==========================================
- Hits 71756 71729 -27
- Misses 83942 83954 +12
Partials 191 191
Flags with carried forward coverage won't be shown. Click here to find out more.
|
@@ -116,10 +116,13 @@ func ensureMigrationUpgrade(tx *pg.Tx) error { | |||
return nil | |||
} | |||
|
|||
func (db *PgDB) readDBCodeAndCheckIfDifferent(dbCodeDir string) (map[string]string, bool, error) { | |||
files, err := os.ReadDir(dbCodeDir) | |||
func (db *PgDB) readDBCodeAndCheckIfDifferent( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really in need of named return values, or a doc comment or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good! My only thought is it'd be nice to check the hash and decide whether to apply changes without having to pass around the hash and bool vars, but that looks difficult to do and I'm ok without it.
Yeah I agree it isn't great. Not sure how to improve it though. |
Ticket
Description
Instead of making
views_and_triggers
use a separate schema just require people to manually drop them.Using a separate schema requires us to set the search path requiring extra permissions. Avoid needing more database permissions with this change.
Test Plan
intg passes
Checklist
docs/release-notes/
.See Release Note for details.