manage admin topic #703
manage admin topic #703
Conversation
marclop
left a comment
marclop
left a comment
There was a problem hiding this comment.
I don't think we should be exposing these primitives in this library, We can fix the issues we're seeing in a different way. For completeness, I've provided some feedback on Slack.
| trace.WithAttributes( | ||
| semconv.MessagingSystemKey.String("kafka"), | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Removing the TODO comment, since I think this is redundant now.
|
|
||
| // Remove cleanup.policy if it exists, | ||
| // since this field cannot be altered. | ||
| delete(c.topicConfigs, "cleanup.policy") |
There was a problem hiding this comment.
Apart from the code refactoring, this is really the only logic change in this PR. cc @marclop
| }, | ||
| }, | ||
| } | ||
|
|
There was a problem hiding this comment.
Test that the cleanup.policy is actually deleted.
| return topics[i].Configs[a].Name < topics[i].Configs[b].Name | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Helper functions to assert configs with multiple fields.
| MeterProvider: mt.MeterProvider, | ||
| }) | ||
| require.NoError(t, err) | ||
|
|
There was a problem hiding this comment.
Moved for readability.
marclop
left a comment
marclop
left a comment
There was a problem hiding this comment.
Thanks for the updates. Let's get this merged.
Add a clear abstraction layer between project and admin topic creation. This PR introduces a
CreateAdminTopicsandCreateProjectTopicsmethod, along with some helper functions to improve code readability.