-
Notifications
You must be signed in to change notification settings - Fork 276
feat(OpenGraph): API - Extension Bundle Upload Endpoint - BED-6723 #2161
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
base: main
Are you sure you want to change the base?
Conversation
# Conflicts: # cmd/api/src/database/graphschema.go
# Conflicts: # cmd/api/src/database/mocks/db.go # cmd/api/src/model/filter.go
# Conflicts: # cmd/api/src/database/graphschema.go # cmd/api/src/database/mocks/db.go
# Conflicts: # cmd/api/src/database/graphschema.go # cmd/api/src/database/graphschema_integration_test.go # cmd/api/src/model/graphschema.go
# Conflicts: # cmd/api/src/database/graphschema_integration_test.go
| routerInst.DELETE(fmt.Sprintf("/api/v2/custom-nodes/{%s}", v2.CustomNodeKindParameter), resources.DeleteCustomNodeKind).RequireAuth(), | ||
|
|
||
| // Open Graph Schema Ingest | ||
| routerInst.PUT("/api/v2/extensions", resources.OpenGraphSchemaIngest).RequireAuth(), |
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.
As this is configuration related, I think it might be a good idea to require a permission related to
"write" for this endpoint
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.
Not sure which permission set would be best, this will be admin only, do you think its worth creating a new set of admin permissions for graph schema management?
# Conflicts: # cmd/api/src/database/graphschema_integration_test.go # cmd/api/src/model/graphschema.go
| // that are *not* in the intersection, and thus must be deleted. | ||
| type MapDiffActions[V any] struct { | ||
| ItemsToDelete []V | ||
| ItemsToUpsert []V |
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.
Do you think we'll ever need an audit on what was created/updated/deleted? I was thinking it could be nice to split upsert into insert/delete?
@superlinkx Any thoughts on if we should be auditing this whole service to help troubleshoot missing fields for whatever reason?
cmd/api/src/model/graphschema.go
Outdated
| SchemaExtensionId int32 `json:"schema_extension_id"` // indicates which extension this edge kind belongs to | ||
| Name string `json:"name"` | ||
| Description string `json:"description"` | ||
| IsTraversable bool `json:"isTraversable"` // indicates whether the edge-kind is a traversable path |
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.
I know you said rough draft but was looking it over and noticed you have this as camelCase instead of snake_case!
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.
Oops haha, these tags are gonna get removed in a bit. Going to re write the models once I get a bit more of the structure updated.
| // that are *not* in the intersection, and thus must be deleted. | ||
| // | ||
| // The OnUpsert func can be used | ||
| func GenerateMapSynchronizationDiffActions[K comparable, V any](src, dst map[K]V, onUpsert func(*V, *V)) MapDiffActions[V] { |
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.
Instead of creating a srcKey map lookup to check if the keys exist, you can simplify this a bit
func GenerateMapSynchronizationDiffActions[K comparable, V any](src, dst map[K]V, onUpsert func(*V, *V)) MapDiffActions[V] {
actions := MapDiffActions[V]{
ItemsToDelete: make([]V, 0),
ItemsToUpsert: make([]V, 0),
}
// 1. Identify keys to delete from the dst
// These will be keys that exist in dst but not in src
for k, v := range dst {
if _, exists := src[k]; !exists {
actions.ItemsToDelete = append(actions.ItemsToDelete, v)
}
}
// 2. Identify keys to upsert (all keys in src)
for k, v := range src {
if onUpsert != nil {
// If the key exists, pass the key, the src value pointer, and the dst value pointer
if dstVal, exists := dst[k]; exists {
onUpsert(&v, &dstVal)
} else {
// If it's a new key, pass nil for the dst value pointer
onUpsert(&v, nil)
}
}
actions.ItemsToUpsert = append(actions.ItemsToUpsert, v)
}
return actions
}
# Conflicts: # cmd/api/src/database/graphschema.go
# Conflicts: # cmd/api/src/database/graphschema.go # cmd/api/src/database/mocks/db.go
Description
Describe your changes in detail
Motivation and Context
Resolves: BED-6723
Why is this change required? What problem does it solve?
How Has This Been Tested?
Please describe in detail how you tested your changes.
Include details of your testing environment, and the tests you ran to
see how your change affects other areas of the code, etc.
Screenshots (optional):
Types of changes
Checklist: