Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

pull magic numbers into constants, update README for 0.6 support

  • Loading branch information...
commit 98c124118ea5e4770c9e848e564f2678cca47a9e 1 parent 7e623a0
@elee elee authored
View
3  Consumer.js
@@ -11,6 +11,7 @@ var Response = require('./Response');
var Consumer = function(options){
this.MAX_MESSAGE_SIZE = 1024 * 1024; // 1 megabyte
+ this.MESSAGE_HEADER_LENGTH = Message.getHeaderLength();
this.DEFAULT_POLLING_INTERVAL = 2; // 2 seconds
this.MAX_OFFSETS = 1;
this.LATEST_OFFSET = -1;
@@ -116,7 +117,7 @@ Consumer.prototype.handleFetchData = function(cb){
var messages = response.messages;
var messageLength = 0;
for (var i = 0; i < messages.length; i++) {
- messageLength += messages[i].payload.length+10;
+ messageLength += messages[i].payload.length + this.MESSAGE_HEADER_LENGTH;
}
this.incrementOffset(messageLength);
this._unsetRequestMode();
View
8 FetchResponse.js
@@ -3,6 +3,8 @@ var BufferMaker = require('buffermaker');
var Message = require('./Message');
var Response = require('./Response');
var _ = require('underscore');
+var RESPONSE_HEADER_LENGTH = Response.getHeaderLength(),
@Raynos Collaborator
Raynos added a note

tiny nitpick: usage of , instead of two var's is a bit weird here. doesn't match local file style.

@elee Collaborator
elee added a note

I struggled with this in my heart of hearts as well. My thought process was "they're not const constants, but I want to group them together visually as vars" -- if you think it's ugly I don't mind changing it :walking:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ MESSAGE_HEADER_LENGTH = Message.getHeaderLength();
/* HEADER
@@ -48,7 +50,7 @@ var FetchResponse = function(error, messages){
this.messages = messages; // an array of message objects
this.length = 2;
_.each(this.messages, function(msg){
- that.length = msg.payload.length+10;
+ that.length = msg.payload.length + MESSAGE_HEADER_LENGTH;
});
this.bytesLengthVal = null;
};
@@ -84,7 +86,7 @@ FetchResponse.fromBytes = function(bytes){
} else {
messages = parseMessages(response.body);
}
- this.bytesLengthVal = response.body.length+6;
+ this.bytesLengthVal = response.body.length + RESPONSE_HEADER_LENGTH;
return new FetchResponse(response.error, messages);
};
@@ -97,7 +99,7 @@ var parseMessages = function(body){
while(body.length > 0){
try {
var message = Message.fromBytes(body);
- body = body.slice(message.payload.length+10);
+ body = body.slice(message.payload.length + MESSAGE_HEADER_LENGTH);
messages.push(message);
} catch(ex){
break;
View
5 Message.js
@@ -6,7 +6,6 @@ var _ = require('underscore');
var BASIC_MESSAGE_HEADER_SIZE = 5;
var MESSAGE_SIZE_BYTE_LENGTH = 4;
var V0_7_MESSAGE_HEADER_SIZE = 10;
-var V0_6_MESSAGE_HEADER_SIZE = 9;
var COMPRESSION_DEFAULT = 0;
var assert = require('assert');
@@ -164,5 +163,9 @@ Message.fromBytes = function(buf){
return new Message(payload, checksum, magic, compression);
};
+Message.getHeaderLength = function() {
+ return V0_7_MESSAGE_HEADER_SIZE;
+};
+
module.exports = Message;
View
2  README.md
@@ -179,7 +179,7 @@ You must also supply a `Callback` to handle any asynchronous
<tr>
<td>Kafka 0.7.0 Release</td><td>Supported</td>
<tr>
- <td>kafka-0.6</td><td>Consumer-only support.</td>
+ <td>kafka-0.6</td><td>Not Supported</td>
<tr>
<td>kafka-0.05</td><td>Not Supported</td>
</table>
View
5 Response.js
@@ -3,6 +3,7 @@ var BufferMaker = require('buffermaker');
var Message = require('./Message');
var _ = require('underscore');
var assert = require('assert');
+var V0_7_RESPONE_HEADER_LENGTH = 6;
/*
0 1 2 3
@@ -78,4 +79,8 @@ Response.prototype.toBytes = function(){
.make();
};
+Response.getHeaderLength = function() {
+ return V0_7_RESPONE_HEADER_LENGTH;
+};
+
module.exports = Response;
@Raynos
Collaborator

tiny nitpick: usage of , instead of two var's is a bit weird here. doesn't match local file style.

@elee
Collaborator

I struggled with this in my heart of hearts as well. My thought process was "they're not const constants, but I want to group them together visually as vars" -- if you think it's ugly I don't mind changing it :walking:

Please sign in to comment.
Something went wrong with that request. Please try again.