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

Implement ucsCallback job #540

Merged
merged 3 commits into from
Nov 8, 2017
Merged

Implement ucsCallback job #540

merged 3 commits into from
Nov 8, 2017

Conversation

bbcyyb
Copy link
Contributor

@bbcyyb bbcyyb commented Nov 2, 2017

Background

UCSM http response is slower than RackHD in some cases and will lead to RackHD fails to get data from UCSM via HTTP. Thus in ucs-service we implemented asynchronous API to avoid this and data will be post back to RackHD via a callback API.

Details

Add new base job for ucs related job.
Unit test is included.

Depends on RackHD/on-http#802, RackHD/ucs-service#28

Paired with: @pengz1
@anhou @pengz1 @mcgG

@anhou anhou added the run-test label Nov 2, 2017
});
};

UcsBaseJob.prototype._subscribeHttpResponseUuidByPromisify = function(id) {
Copy link
Member

Choose a reason for hiding this comment

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

Good job!

Copy link
Member

Choose a reason for hiding this comment

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

_subscribeHttpResponseUuid is already a promise(https://github.com/RackHD/on-tasks/blob/master/lib/jobs/base-job.js#L230), we don't need to promisify it again.

Copy link
Member

Choose a reason for hiding this comment

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

OK, since you want to Repromisify _subscribeHttpResponseUuid, errors should be processed by calling callback.

Copy link
Contributor Author

@bbcyyb bbcyyb Nov 6, 2017

Choose a reason for hiding this comment

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

Yes, current code will eat all error scenes. I'm going to add error handler for asynchronous processing, to make sure errors can be found and handled.

});
};

UcsBaseJob.prototype._subscribeHttpResponseUuidByPromisify = function(id) {
Copy link
Member

Choose a reason for hiding this comment

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

_subscribeHttpResponseUuid is already a promise(https://github.com/RackHD/on-tasks/blob/master/lib/jobs/base-job.js#L230), we don't need to promisify it again.

@bbcyyb
Copy link
Contributor Author

bbcyyb commented Nov 6, 2017

Change parameter from taskId to a uuid string named callbackId which is used to define a amqp channel.

@anhou anhou merged commit 3c44736 into RackHD:master Nov 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants