feat: support FilterPolicy in worker service#3338
Conversation
|
|
||
| type stringOrInterface struct { | ||
| String *string | ||
| Interface map[string]interface{} |
There was a problem hiding this comment.
If we have just:
type FilterPolicy map[string]interface{}
Will the marshaling to JSON work if the level of nesting in the filter policy is more than 1-level? https://stackoverflow.com/a/40737676/1201381
There was a problem hiding this comment.
| if err != nil { | ||
| return nil, fmt.Errorf(`convert "filter_policy" to a JSON string: %w`, err) | ||
| } | ||
| if string(bytes) == "null" { |
There was a problem hiding this comment.
wut lol 😂
can we instead add a guard clause at the top of the function:
if filterPolicy == nil {
return nil, nil
}There was a problem hiding this comment.
it's in their codebase lol
There was a problem hiding this comment.
hmm unfortunately we have to do that without pointer because of the mergo issue. maybe i can check the length of the map
| func convertTopicSubscription(t manifest.TopicSubscription, url, accountID, app, env, svc string) ( | ||
| *template.TopicSubscription, error) { |
There was a problem hiding this comment.
super tiny nit: maybe
func convertTopicSubscription(t manifest.TopicSubscription, url, accountID, app, env, svc string)
(*template.TopicSubscription, error) {or
func convertTopicSubscription(t manifest.TopicSubscription, url, accountID, app, env, svc string (*template.TopicSubscription, error) {Edit: ignore this this is outdated now
| Queue *SQSQueue | ||
| Name *string | ||
| Service *string | ||
| FilterPolicy *string |
There was a problem hiding this comment.
Just to call it out here: I think we are probably fine with string instead of *string for a lot of these variables because "" being a string of length of 0 will result in false when evaluated in a if statement
Addresses #3308
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.