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

createTopic error restructure #1104

Merged
merged 4 commits into from
May 24, 2021
Merged

Conversation

prithvijit-dasgupta
Copy link
Contributor

@prithvijit-dasgupta prithvijit-dasgupta commented May 18, 2021

As a trial for #957

  1. Added KafkaJSAggregateError (following AggregateError) and KafkaJSCreateTopicError (extends KafkaJSProtocolError)
  2. Moved createTopic v1 parser to common v0 parser as both are equivalent
  3. Modified admin API to deal with new createTopic error structure

P.S. Will be fixing spec and test files

@Nevon
Copy link
Collaborator

Nevon commented May 22, 2021

This looks great. I think following the AggregateError spec is a good choice. Once you've fixed the issues with the tests, I'd be happy to merge this contribution.

@prithvijit-dasgupta
Copy link
Contributor Author

@Nevon fixed the tests. Can I start with restructuring Consumer API protocol errors as my team is more interested in the consumers?

@Nevon Nevon merged commit 7292734 into tulios:master May 24, 2021
@Nevon
Copy link
Collaborator

Nevon commented May 24, 2021

Before you start, I would just ask that you outline what scenarios you'd like to address in the linked issue. It's also good if you could make separate PRs for each scenario if you plan on addressing multiple ones - makes it easier to review

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