-
Notifications
You must be signed in to change notification settings - Fork 25
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
Feature: Support store Policy - SQL scheme #234
Conversation
Signed-off-by: Kairo Araujo <[email protected]>
Include the `go generate ./...` in the db migrations steps `make db-migrations` now generates the ent files from schemes. Signed-off-by: Kairo Araujo <[email protected]>
Adds the basic schema for the policy Signed-off-by: Kairo Araujo <[email protected]>
) | ||
|
||
// Attestation represents an attestation from a witness attestation collection | ||
type AttestationPolicy struct { |
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.
Policy
is a reserved name by ent
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #234 +/- ##
===========================================
- Coverage 82.40% 68.19% -14.22%
===========================================
Files 10 11 +1
Lines 358 503 +145
===========================================
+ Hits 295 343 +48
- Misses 43 105 +62
- Partials 20 55 +35 ☔ View full report in Codecov by Sentry. |
4a1cd88
to
30f6b62
Compare
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 to me! This is a great step towards full policy support. 🥳 I'll let @mikhailswift for the final approval. Just had a question or two from my ignorance of ent
. 😅
ent/schema/statement.go
Outdated
@@ -38,6 +38,7 @@ func (Statement) Fields() []ent.Field { | |||
func (Statement) Edges() []ent.Edge { | |||
return []ent.Edge{ | |||
edge.To("subjects", Subject.Type).Annotations(entgql.RelayConnection()), | |||
edge.To("policies", AttestationPolicy.Type).Annotations(entgql.RelayConnection()), |
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 a question for someone who knows more about ent
than I do but, if policies -> statements are a 1:1 mapping, do we need the RelayConnection()
? Maybe related, should we call the edge the singular policy
?
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.
Yes, it makes sense.
I will wait for @mikhailswift, so if I need to make more changes :)
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.
The general way I think about whether an edge should be a RelayConnection is whether you want to query that edge directly or use it as an entry to the graph. We don't do so for the attestation_collections
edge below, so I'd be inclined to keep it similar. Similarly to that, we probably want the policies
edge to be Unique
, since they are 1:1.
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.
Thanks!
I have added the new commit 9b7918e
Signed-off-by: Kairo Araujo <[email protected]>
04e6d30
to
44bcd98
Compare
This commit includes the db generate for policy and the db migrations `make db-migrations` Signed-off-by: Kairo Araujo <[email protected]>
44bcd98
to
6756133
Compare
Hey @jkjell, @mikhailswift 👋 |
What this PR does / why we need it
This is the first part of feature #158
This PR adds the schema in the
ent
to support the basic information to store Policy in the SQL StorageNext:
Which issue(s) this PR fixes (optional)
Related to #158
Fixes #167 and #169
Acceptance Criteria Met
Note for reviewers:
The review can focus on the three first commits, the last commit are db
generate
andmigrations