Skip to content
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

add topicConfiguration property to kafka channel binding #163

Closed
thake opened this issue Oct 24, 2022 · 3 comments · Fixed by #171
Closed

add topicConfiguration property to kafka channel binding #163

thake opened this issue Oct 24, 2022 · 3 comments · Fixed by #171
Labels
enhancement New feature or request

Comments

@thake
Copy link
Contributor

thake commented Oct 24, 2022

Reason/Context

Many topic configuration properties influence how API consumers can use a topic that is owned by an API provider. To transparently communicate relevant topic configurations, it should be part of the AsyncAPI spec.

Example:
If the cleanup.policy and the related configurations delete.retention.ms, retention.ms, and retention.bytes were available in the AsyncAPI spec, an API consumer could at least conduct the following:

  • Longest possible time to recover from a failure
  • That messages can get lost due to compaction.

Description

Add a new topicConfiguration property to the kafka channel binding. API providers can add relevant configuration properties to the API-spec.

topicConfiguration:
  description: Topic configuration properties that are relevant to the consumer.
  type: object
  additionalProperties: true  

By adding a generic topicConfiguration property, the kafka binding stays generic to configuration evolution in kafka itself.

@thake thake added the enhancement New feature or request label Oct 24, 2022
@github-actions
Copy link

Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@dalelane
Copy link
Collaborator

I like the idea - I can see how knowing the retention config will be useful from a dev-ops perspective for an application.

We should make sure we name and structure the variables in a way that will be as future-proof as possible - e.g. do we keep it flat and put retention config values alongside the existing partitions and replicas values or do we nest it under a configuration key. (FWIW, I'm leaning towards flat, but I'm open to pros/cons either way)

But naming specifics-aside, I think the principle of adding retention to the bindings is a good one.

@thake Would you like to make a pull request with a proposed structure?

@thake
Copy link
Contributor Author

thake commented Nov 7, 2022

@dalelane, thanks for picking this issue up 👍. I'm happy to work out a PR, once we've decided on the rough structure.

Independent of the structure, I think the following configuration parameters have an influence on an application's API if they publish on a kafka topic:

My intention with adding a deeper nested structure was that the AsyncAPI binding does not need to be updated/synced with the list of configuration options for a kafka topic. Whenever a new configuration option is added to a topic in kafka, it can be used in AsyncAPI, without needing to adapt the Binding. However, as only roughly 5 of the ~20 configuration options have any relevance for the AsyncAPI spec, I'm also fine with adding the options explicitly to the AsyncAPI binding. This would allow us to use a flat structure next to partitions.

Before creating a PR, is there any naming convention for the property names in AsyncAPI? I've only seen camelCase properties in the spec. I wondered if it would be possible to use the configuration properties as is (e.g., cleanup.policy instead of cleanupPolicy) to reduce brain overload/complexity for writers/readers/tool-writers. Because of the clash of naming conventions, this would IMHO better fit a deep nesting structure.

Taking all that into account, my current proposal would be the following:

topicConfiguration:
  description: Topic configuration properties that are relevant to the consumer.
  type: object
  properties:
    cleanup.policy:
      description: The [`cleanup.policy`](https://kafka.apache.org/documentation/#topicconfigs_cleanup.policy) configuration option.
      type: array
      items: 
        type: string
        enum: [compact, delete]
    retention.ms:
      description: The `retention.ms`](https://kafka.apache.org/documentation/#topicconfigs_retention.ms) configuration option.
      type: integer
      minimum: -1
    ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants