Skip to content

add new feature to make sure we aren't writing PII into the application ... - #2668

Merged
chrisndodge merged 1 commit into
masterfrom
cdodge/no-pii-in-logs
Feb 26, 2014
Merged

add new feature to make sure we aren't writing PII into the application ...#2668
chrisndodge merged 1 commit into
masterfrom
cdodge/no-pii-in-logs

Conversation

@chrisndodge

Copy link
Copy Markdown
Contributor

...logs. So far we're aware of doing so regarding Auth use-cases

Comment thread cms/envs/common.py Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

typo: Identifiable

@chrisndodge

Copy link
Copy Markdown
Contributor Author

@jbau @jzoldak thanks for the input. I think I reconciled your feedback.

Jason, can you quickly eyeball again. There was a merge conflict when I had to rebase, plus Jay asked me to convert over the " .... %s ...." string formatting to use .format() instead.

@chrisndodge

Copy link
Copy Markdown
Contributor Author

OK, spoke too soon. Need to update shib_test since the change to .format() changed the number of arguments into the logging function.

@chrisndodge

Copy link
Copy Markdown
Contributor Author

@jbau @jzoldak fixed up the tests. This should be ready for final eyeballing.... Thx.

Comment thread common/djangoapps/student/views.py Outdated

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think user_found_by_email_lookup can't be None here, but it's not readily apparent. Is it possible to assuage my paranoia and explicitly handle that case?

@chrisndodge

Copy link
Copy Markdown
Contributor Author

@jbau better?

@jbau

jbau commented Feb 25, 2014

Copy link
Copy Markdown

yes, better.

👍

…on logs. So far we're aware of doing so regarding Auth use-cases
@chrisndodge

Copy link
Copy Markdown
Contributor Author

@jzoldak can I get your thumbs up as well? Or should I get another reviewer, like @dianakhuang or someone else from LMS team?

@jzoldak

jzoldak commented Feb 26, 2014

Copy link
Copy Markdown
Contributor

👍

chrisndodge pushed a commit that referenced this pull request Feb 26, 2014
add new feature to make sure we aren't writing PII into the application ...
@chrisndodge
chrisndodge merged commit 38e55a4 into master Feb 26, 2014
@jzoldak
jzoldak deleted the cdodge/no-pii-in-logs branch May 5, 2014 14:54
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Jun 21, 2019
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