Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Log the hostname the reCAPTCHA was completed on #946

Merged
merged 2 commits into from
Jul 25, 2016

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Jul 22, 2016

This could be useful information to have in the logs. Also comment about how & why we don't verify the hostname.

This could be useful information to have in the logs. Also comment about how & why we don't verify the hostname.
logger.info(
"%s reCAPTCHA from hostname %s",
"Successful" if resp_body['success'] else "Failed",
resp_body['hostname']
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use .get to default the 'hostname' key here?

 resp_body.get('hostname', "<missing hostname key>")

@NegativeMjark
Copy link
Contributor

NegativeMjark commented Jul 22, 2016

LGTM other than the possibility for KeyErrors
(Which is probably what's causing the test failures)

@dbkr
Copy link
Member Author

dbkr commented Jul 22, 2016

Done. Are these tests still 'expected' to fail?

@NegativeMjark
Copy link
Contributor

Done. Are these tests still 'expected' to fail?

No, but they're now failing due to changes that were snuck in while the CI was broken :(.

@NegativeMjark
Copy link
Contributor

LGTM

@dbkr dbkr merged commit 4fcdf7b into develop Jul 25, 2016
@richvdh richvdh deleted the dbkr/log_recaptcha_hostname branch December 1, 2016 14:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants