[POR-1616] [POR-1469] [POR-1881] [POR-2026] [POR-1451] [POR-1766] Backend changes to support notifications #3921
Conversation
ferozemohideen
changed the title
| if err := repo.db.Where("porter_app_id = ? AND type = 'NOTIFICATION' AND metadata->>'app_revision_id' = ?", strAppID, appRevisionId).Find(¬ifications).Error; err != nil { | ||
| return notifications, err |
There was a problem hiding this comment.
Probably not needed now but could end up being nice just to have app_revision_id as its own column on events
…notifications-backend
…notifications-backend
…notifications-backend
…notifications-backend
…notifications-backend
…notifications-backend
| return nil // nothing to update here | ||
| } | ||
|
|
||
| serviceStatus, ok := matchEvent.Metadata["service_deployment_metadata"] |
There was a problem hiding this comment.
Can we make the serviceDeploymentGenericMap a type? If not because of dynamic keys, is there a better way we can represent that data?
If the goal here is just to sort the data, can we move that to an auxiliary function to make this more clear
There was a problem hiding this comment.
the service deployment metadata would be handled much easier if it was a list type, but changing that would require changing the way we publish it in ccp
Filing a ticket for making that refactor, after which we can simplify this code: https://linear.app/porter/issue/POR-2101/turn-servicedeploymentmetadata-from-a-map-into-a-list-in-ccp
| return serviceDeploymentMetadata, telemetry.Error(ctx, span, nil, "event is not a deploy event") | ||
| } | ||
|
|
||
| serviceStatus, ok := deployEvent.Metadata["service_deployment_metadata"] |
There was a problem hiding this comment.
Similar to the above - can we change the structure, or pull this code out?
| return telemetry.Error(ctx, span, err, "deployment target id cannot be nil") | ||
| } | ||
|
|
||
| notificationMap := make(map[string]any) |
There was a problem hiding this comment.
You can drop this marshalling with the JSONB part also. Test it out to be sure so gorm has no issues
| @@ -0,0 +1,281 @@ | |||
| package porter_error | |||
There was a problem hiding this comment.
None of this is a requested change, just a suggestion for when dealing with this type of code again.
This is a good implementation but relies on never changing error messages from the agent side. This is where having a distributed contract for the errors which can be returned, which can be read into the agent, or Porter is massively helpful (like an api contract, not just in the porter-dev/api-contracts proto sense)
Since a lot of this code is copy-pasta with minor changes, that usually is a good case for code generation based on a yaml, toml, etc.
Protobuf handles this with messages, grapqhl has error extensions, open api has error definitions which would allow you to consolidate this if needed.
…notifications-backend
…notifications-backend
…notifications-backend
ferozemohideen
changed the title
ferozemohideen
changed the title
POR-1616 POR-1469 POR-1881 POR-2026
What does this PR do?
These changes are introduced to reduce user confusion when it comes to APP_EVENT type events. Instead of changing the agent code to send better app events, we now convert app events to the new Notification struct on the Porter server side.
I would recommend looking at the changes in create_and_update_events.go first, which then brings the logic to the internal/notifications package. From there, you can follow the steps of HandleNotification in notification.go