feat(event-manager): Adds implementation for ODPEventManager #354
Conversation
pkg/odp/event_manager.go
Outdated
| const DefaultEventQueueSize = 10000 | ||
|
|
||
| // DefaultEventFlushInterval holds the default value for the event flush interval | ||
| const DefaultEventFlushInterval = 30 * time.Second |
There was a problem hiding this comment.
| const DefaultEventFlushInterval = 30 * time.Second | |
| const DefaultEventFlushInterval = 1 * time.Second |
Consistent (flush interval = 1sec) with other SDKs
pkg/odp/event_manager.go
Outdated
| odpEvent.Data["idempotence_id"] = guuid.New().String() | ||
| odpEvent.Data["data_source_type"] = "sdk" | ||
| odpEvent.Data["data_source"] = event.ClientName | ||
| odpEvent.Data["data_source_version"] = event.Version |
There was a problem hiding this comment.
Client-provided data has higher priority if it has key-conflict with common-data.
For example, if "data_source" data provided in the data, it should not be overwritten to "go-sdk".
There was a problem hiding this comment.
Can we have a unit test to cover this case?
pkg/utils/utils.go
Outdated
| if len(a) != len(b) { | ||
| return false | ||
| } | ||
| for i, v := range a { |
There was a problem hiding this comment.
This will return false for same array values with different order. Wondering if we're ok for all use cases.
There was a problem hiding this comment.
good catch, fixing this.
pkg/odp/event_api_manager.go
Outdated
| func (s *DefaultEventAPIManager) SendODPEvents(config Config, events []Event) (canRetry bool, err error) { | ||
|
|
||
| // Creating request | ||
| apiEndpoint := config.GetAPIHost() + eventsAPIEndpointPath |
pkg/odp/event_api_manager.go
Outdated
|
|
||
| // Creating request | ||
| apiEndpoint := config.GetAPIHost() + eventsAPIEndpointPath | ||
| headers := []utils.Header{{Name: "Content-Type", Value: "application/json"}, {Name: ODPAPIKeyHeader, Value: config.GetAPIKey()}} |
|
|
||
| // Start does not do any initialization, just starts the ticker | ||
| func (bm *BatchEventManager) Start(ctx context.Context) { | ||
| if !bm.IsOdpServiceIntegrated() { |
There was a problem hiding this comment.
I think this should be ready? please check in other sdks.
There was a problem hiding this comment.
This is similar to odpServiceIntegrated property in swift-sdk
pkg/odp/segment_api_manager.go
Outdated
| // Creating request | ||
| apiEndpoint := config.GetAPIHost() + "/v3/graphql" | ||
| headers := []utils.Header{{Name: "Content-Type", Value: "application/json"}, {Name: "x-api-key", Value: config.GetAPIKey()}} | ||
| apiEndpoint := config.GetAPIHost() + graphqlAPIEndpointPath |
| s.NoError(err) | ||
| s.Empty(segments, "none of the test segments in the live ODP server") | ||
| } | ||
| // func (s *SegmentAPIManagerTestSuite) TestLiveOdpGraphQL() { |
There was a problem hiding this comment.
is this intentional?
There was a problem hiding this comment.
Yes, have put these here for debug purposes similar to swift-sdk
pkg/odp/segment_manager.go
Outdated
| ) | ||
|
|
||
| // SMOptionConfig are the SegmentManager options that give you the ability to add one more more options before the segment manager is initialized. | ||
| type SMOptionConfig func(em *DefaultSegmentManager) |
There was a problem hiding this comment.
shouldn't this be SMConfigOptions
There was a problem hiding this comment.
Changing it into SMOptionFunc which is in sync with the older options in the sdk.
|
|
||
| func TestCompareSlices(t *testing.T) { | ||
| assert.True(t, CompareSlices(nil, nil)) | ||
| assert.True(t, CompareSlices([]string{}, []string{})) |
There was a problem hiding this comment.
can you try
make and []string{}
3 participants
Summary
This PR adds support for ODPEventManager.
Test plan
Issues