Skip to content
This repository has been archived by the owner on Sep 13, 2022. It is now read-only.

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
Signed-off-by: Won Jun Jang <wjang@uber.com>
  • Loading branch information
black-adder committed Apr 10, 2018
1 parent 0b1620f commit 86a03ed
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 12 deletions.
4 changes: 2 additions & 2 deletions src/samplers/remote_sampler.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,10 @@ export default class RemoteControlledSampler {

_refreshSamplingStrategy() {
let serviceName: string = encodeURIComponent(this._serviceName);
let success: Function = body => {
const success: Function = body => {
this._parseSamplingServerResponse(body);
};
let error: Function = err => {
const error: Function = err => {
this._logger.error(`Error in fetching sampling strategy: ${err}.`);
this._metrics.samplerQueryFailure.increment(1);
};
Expand Down
2 changes: 1 addition & 1 deletion src/throttler/default_throttler.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
export default class DefaultThrottler {
_throttleAll: boolean;

constructor(throttleAll: ?boolean) {
constructor(throttleAll?: boolean) {
this._throttleAll = throttleAll || false;
}

Expand Down
2 changes: 1 addition & 1 deletion src/throttler/remote_throttler.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ export default class RemoteThrottler {
}
}

close(callback: ?Function): void {
close(callback?: Function): void {
clearTimeout(this._initialDelayTimeoutHandle);
clearInterval(this._refreshIntervalHandle);

Expand Down
10 changes: 5 additions & 5 deletions test/lib/config_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export default class ConfigServer {
constructor(port: number = 5778) {
this._port = port;
this._app = express();
this._strategies = Object.create(null);
this._strategies = {};
this._app.get('/sampling', this._handleSampling.bind(this));
this._app.get('/credits', this._handleThrottling.bind(this));
}
Expand All @@ -37,8 +37,8 @@ export default class ConfigServer {
}

clearConfigs(): void {
this._strategies = Object.create(null);
this._credits = Object.create(null);
this._strategies = {};
this._credits = {};
}

_handleSampling(req: any, res: any) {
Expand All @@ -54,8 +54,8 @@ export default class ConfigServer {
}

_handle(req: any, res: any, getFunc: Function) {
let service = req.query.service;
let resp = getFunc(service);
const service = req.query.service;
const resp = getFunc(service);
if (resp) {
res.send(resp);
} else {
Expand Down
8 changes: 5 additions & 3 deletions test/throttler/remote_throttler.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ describe('RemoteThrottler should', () => {
let operation = 'op';
let other_operation = 'oop';
let uuid = 'uuid';
let creditsUpdatedHook;

before(() => {
server = new ConfigServer().start();
Expand All @@ -53,6 +54,7 @@ describe('RemoteThrottler should', () => {
initialDelayMs: 60000,
metrics: metrics,
logger: logger,
onCreditsUpdate: (...args) => creditsUpdatedHook(...args),
});
});

Expand All @@ -63,7 +65,7 @@ describe('RemoteThrottler should', () => {
it('return false for isAllowed on initial call and return true once credits are initialized', done => {
throttler.setProcess({ uuid: uuid });
server.addCredits(serviceName, [{ operation: operation, credits: 3 }]);
throttler._onCreditsUpdate = _throttler => {
creditsUpdatedHook = _throttler => {
assert.isOk(_throttler.isAllowed(operation));
assert.equal(_throttler._credits[operation], 2);
assert.equal(LocalBackend.counterValue(metrics.throttlerUpdateSuccess), 1);
Expand Down Expand Up @@ -106,7 +108,7 @@ describe('RemoteThrottler should', () => {
]);
throttler._credits[operation] = 0;
throttler._credits[other_operation] = 0;
throttler._onCreditsUpdate = _throttler => {
creditsUpdatedHook = _throttler => {
assert.isOk(_throttler.isAllowed(operation));
assert.equal(_throttler._credits[operation], 4);
assert.isOk(_throttler.isAllowed(other_operation));
Expand Down Expand Up @@ -160,7 +162,7 @@ describe('RemoteThrottler should', () => {
throttler.setProcess({ uuid: uuid });
throttler._credits[operation] = 0;
server.addCredits(serviceName, [{ operation: operation, credits: 5 }]);
throttler._onCreditsUpdate = _throttler => {
creditsUpdatedHook = _throttler => {
assert.isOk(_throttler.isAllowed(operation));
assert.equal(_throttler._credits[operation], 4);
assert.equal(LocalBackend.counterValue(metrics.throttlerUpdateSuccess), 1);
Expand Down

0 comments on commit 86a03ed

Please sign in to comment.