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

Add name to event data #2

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add name to event data #2

wants to merge 4 commits into from

Conversation

y-tang
Copy link
Collaborator

@y-tang y-tang commented Jul 30, 2015

@shelbys @pikajude review please


req = self.httpClient.request(location, xml, function(err, response, body) {
var result;
var obj;
self.lastResponse = body;
self.lastResponseHeaders = response && response.headers;
self.emit('response', body);
self.emit('response', body, name);
Copy link
Owner

Choose a reason for hiding this comment

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

Shouldn't we also combine this and above into soapSuccess, so the request and response are correlated into one entry?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adding a new event soapSuccess? We could. Then we only need to handle 2 events. But I assume we don't want to remove any existing events for backward compatibility.

@shelbys
Copy link
Owner

shelbys commented Jul 31, 2015

@y-tang - How does this fit in api-gateway? I don't see any equivalent changes in there

@y-tang
Copy link
Collaborator Author

y-tang commented Jul 31, 2015

@shelbys I have not updated api-gateway yet. I thought about registering event handlers in ApiGateway.coffee for each data source after they are connected. But I don't have access to request specific log there. Is it ok to use the global log? If not, the only way I can think of is to register in the before hook and remove the listener in the after hook for each request. But that seems a little extreme.

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.

3 participants