-
Notifications
You must be signed in to change notification settings - Fork 68
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
fix for hide bugzilla user & password in any and all output from scri… #35
Conversation
Could you please reuse the same Pull Request instead of recreating a new one ? git push -f is enough to force the push. |
try: | ||
return BugSearch.get(url).bugs | ||
except Exception, e: | ||
pattern = re.compile( |
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.
The code is duplicated, i think we can avoid that.
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.
@sylvestre: I have moved as a separate function in utils.
@sylvestre I have changed the function name. Looks like Exception always provide a message. But for the safer side, I am doing str(error) now( previously we were printing directly the error in console) |
Looks great, could you just write a unit test for hide_personal_info? |
@sylvestre Sure. I will write unit test |
Thanks! Please note that I just moved the files into a different directory (ditto for the tests)... really sorry! |
@sylvestre Added test case as well |
"changed_field=status&changed_after=2010-12-24&" + \ | ||
"include_fields=_default,attachments&changed_field_to=" + \ | ||
"RESOLVED&api_key=xyzxyzxyz&resolution=FIXED" | ||
hide_personal_info(sample_exception) |
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.
To be complete, we should check that the message does not contain "xyzxyzxyz"
In this case, probably something like
assert "xyzxyzxyz" not in hide_personal_info(sample_exception)
should do the work
@sylvestre: Done! |
I am going to merge it but to be complete, we should also test get_bug with this change: |
Fix for hide bugzilla user & password in any and all output from scri…
When ever an error occurs, From /usr/local/lib/python2.7/dist-packages/remoteobjects/http.py, error will get raised using err_cls. Example:
raise err_cls('%d %s requesting %s %s'
response.status, response.reason, classname, url))
We will replace such error urls's api_key with *** while printing in console.
Now sample error response will be:
File "test.py", line 21, in
buglist = bmo.get_bug_list(options)
File "/home/bztools/bugzilla/agents.py", line 46, in get_bug_list
raise Exception(error)
Exception: 400 Bad Request requesting BugSearch https://bugzilla.mozilla.org/bzapi/bug/?&changed_before=2010-12-26&product=Core,Firefox&changed_field=status&changed_after=2010-12-24&include_fields=_default,attachments&changed_field_to=RESOLVED&api_key=*****************************************&resolution=FIXED