-
Notifications
You must be signed in to change notification settings - Fork 170
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
THREESCALE-10894 concat filtered services into a single log #1493
Conversation
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.
Good job on the unittest 👍 , can we also add the integration test?
And for integration test, |
location ~ / { | ||
echo 'yay, api backend'; | ||
} | ||
--- pipelined_requests eval |
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.
Use request
instead. You only send one request here, and no need for eval
--- request
GET /?user_key=1
["GET /?user_key=1"] | ||
--- more_headers eval | ||
["Host: one"] | ||
--- response_body eval | ||
["yay, api backend\n"] | ||
--- error_code eval | ||
[200] | ||
--- error_log eval | ||
[qr/filtering out services: 11, 33, 999/] |
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.
Same as above, remove eval
--- request
GET /?user_key=1
--- more_headers
Host: one
--- response_body
yay, api backend
--- error_code: 200
--- error_log
filtering out services: 11, 33, 999
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.
@tkan145 Just curious what does the eval
keyword do here and why is it not needed?
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.
LGTM,
Once we have all the CI tests passed, we can merge this
c831397
to
a8987f6
Compare
Jira: https://issues.redhat.com/browse/THREESCALE-10894
Verification:
When the APIcast boots up, confirm the logs to be:
Fitelered out services should display your ProductC and ProductD IDs