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

Allow creating topic using topic description from file #210

Closed

Conversation

gotha
Copy link
Contributor

@gotha gotha commented Aug 10, 2024

Description

Allow the creation of a topic using configuration description from file

kafkactl describe topic my-topic -o json > my-topic-config.json
kafkactl create topic my-topic-clone --file my-topic-config.json

Note that in the current implementation configuration from file would override the configuration provided via flag.
For example, if the file specifies that the topic has 10 partitions even if --partition=5 flag is set, 10 partitions will be created.
This might be confusing for the user.

I could have implemented the reading of the config file to happen in cmd/create/create-topic.go so the flags can override the configuration from the file, but it looked that it would be a bigger change in the project structure.
I am happy to change it so flags override config file values if you think this is a better approach.

We can also return an error if some flags and --file flag are provided together, so it wont allow the user to provide --partition and --file simultaneously.
But then we have the same problem with --config and --file, unless we want to forbid the user to provide --config flag (which will unnecessarily limit the functionality), we should establish order of precedence - flag over file or vice versa.

Related to issue #202

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Documentation

  • the change is mentioned in the ## [Unreleased] section of CHANGELOG.md
  • the configuration yaml was changed and the example config in README.adoc was updated
  • a usage example was added to README.adoc
  • tests for the changes have been implemented (see: Testing your changes)

@gotha gotha force-pushed the feature/create-topic-from-file branch from bc4e98b to 72dd004 Compare August 10, 2024 21:07
}

topicDetails.NumPartitions = int32(len(createTopicConfig.Partitions))
topicDetails.ReplicationFactor = int16(len(createTopicConfig.Partitions[0].Replicas))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not super sure about the ReplicationFactor...
Maybe if it cannot be reliably determined, it should be removed.

@d-rk
Copy link
Collaborator

d-rk commented Aug 14, 2024

I updated some stuff on this PR but you did not gave me rights to push to your branch, so I'm closing this in favor of #212

@d-rk d-rk closed this Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants