-
Notifications
You must be signed in to change notification settings - Fork 627
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 create topics support #958
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
92b835d
Add create topics support
jlandersen b7e53c5
Handle different error message in kafka versions
jlandersen de03d2d
Remove metadata api support version
jlandersen 1c46c1b
Return error if controller cannot be found
jlandersen d91091b
Parse partition metadata correctly for v1
jlandersen d904075
Address PR feedback
jlandersen 038c903
Encode null value properly
jlandersen 2916d70
Forward old createTopics calls to previous impl
jlandersen 0ec81c8
Update README with new createTopics function
jlandersen 59052d5
Update cached metadata in getController
jlandersen d039f44
Remove unused sandbox
jlandersen 911e3e2
Add additional assertion of createTopic test
jlandersen ac6d07e
Add sendControllerRequest method
jlandersen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
var util = require('util'); | ||
|
||
/** | ||
* The request was sent to a broker that was not the controller. | ||
* | ||
* @param {*} message A message describing the issue. | ||
* | ||
* @constructor | ||
*/ | ||
var NotController = function (message) { | ||
Error.captureStackTrace(this, this); | ||
this.message = message; | ||
}; | ||
|
||
util.inherits(NotController, Error); | ||
NotController.prototype.name = 'NotController'; | ||
|
||
module.exports = NotController; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @jlandersen thanks for the update. Could you help me understand why
wrapControllerCheckIfNeeded
is needed? Would it not be simpler to just inline a function in place of thecallback
that checks if the error is an instance ofNotControllerError
and then set the controller Id to null and callsendControllerRequest
again if it is?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason was to limit the retry to a single time (or potentially x times). If we add a function in place of the callback that always calls the
sendControllerRequest
on aNotControllerError
, it could keep making requests if the error is returned repeatedly. I am not saying it can or will happen, but was rather to be safe. Let me know if you prefer otherwise and i'll put the change in!