Update compliance database schema #157
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JustinKuli
reviewed
Dec 21, 2023
controllers/complianceeventsapi/migrations/000001_compliance_history_initial_tables.up.sql
Outdated
Show resolved
Hide resolved
JustinKuli
reviewed
Jan 2, 2024
controllers/complianceeventsapi/migrations/000001_compliance_history_initial_tables.up.sql
Outdated
Show resolved
Hide resolved
|
@JustinKuli the latest commit makes the SQL queries static. Let me know what you think of this approach! |
JustinKuli
reviewed
Jan 4, 2024
Comment on lines
+472
to
+374
| // getOrCreate will translate the input object to an INSERT SQL query. When the input object already exists in the | ||
| // database, a SELECT query is performed. The primary key is set on the input object when it is inserted or gotten | ||
| // from the database. The INSERT first then SELECT approach is a clean way to account for race conditions of multiple | ||
| // goroutines creating the same row. |
There was a problem hiding this comment.
Neat! I don't think I fully appreciated this on my first review.
JustinKuli
previously approved these changes
Jan 4, 2024
This adds a namespace field to the parent policy table so that the root policy namespace and name is recorded instead of the replicated policy name which includes the root policy namespace in it. This is more intuitive since the user interacts with the root policy and not often with the replicated policy. This also moves the parent policy foreign key to the compliance_events table. This is better as a point in time record of the parent policy associated with the policy that generated the compliance event. It does make filtering more efficient later to see any compliance event associated with a parent policy. It also allows less data duplication on the policies table. As part of this refactoring, goku was removed in favor of a minimal translation layer of database queries to structs and vice versa. Signed-off-by: mprahl <[email protected]>
This simplifies the logic and makes it clearer what query is being made when reading the code. Signed-off-by: mprahl <[email protected]>
This also removes the unused fromRow. Signed-off-by: mprahl <[email protected]>
|
@JustinKuli alright, all the complicated "cool" stuff has been removed from the PR now. 😆 I kept them in the commit history in case we decided to go back that route. |
JustinKuli
approved these changes
Jan 5, 2024
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JustinKuli, mprahl The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/unhold Thanks (and sorry) for all the back-and-forth, but I think this looks really great now |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a namespace field to the parent policy table so that the root policy namespace and name is recorded instead of the replicated policy name which includes the root policy namespace in it. This is more intuitive since the user interacts with the root policy and not often with the replicated policy.
This also moves the parent policy foreign key to the compliance_events table. This is better as a point in time record of the parent policy associated with the policy that generated the compliance event. It does make filtering more efficient later to see any compliance event associated with a parent policy. It also allows less data duplication on the policies table.
As part of this refactoring, goku was removed in favor of a minimal translation layer of database queries to structs and vice versa.