bzr-xmloutput

Merge lp://qastaging/~piastucki/bzr-xmloutput/fix-auth into lp://qastaging/bzr-xmloutput

  1. fix-auth
  2. Merge into trunk
Proposed by Piotr Piastucki
Status: Merged
Approved by: Guillermo Gonzalez
Approved revision: 168
Merged at revision: 167
Proposed branch: lp://qastaging/~piastucki/bzr-xmloutput/fix-auth
Merge into: lp://qastaging/bzr-xmloutput
Diff against target: 230 lines (+161/-9)
5 files modified
service.py (+17/-9)
tests/__init__.py (+2/-0)
tests/test_auth_service.py (+68/-0)
tests/test_uifactory.py (+23/-0)
uifactory.py (+51/-0)
To merge this branch: bzr merge lp://qastaging/~piastucki/bzr-xmloutput/fix-auth
Reviewer Review Type Date Requested Status
Guillermo Gonzalez Approve
Review via email: mp+150190@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2013-02-16.

Description of the change

This commit introduces a custom UIFactory that handles get_password() and get_username() to avoid XMLRPC server hanging on waiting for user's input.
When 'run_bzr' function is invoked ValueError will be thrown whenever get_password() or get_username() is called to avoid server hang and to allow the client to ask the user for credentials.
Additionally, a new function 'run_bzr_auth' was added to make it possible to pass username and password to be used instead of throwing the error.

To post a comment you must log in.
Revision history for this message
Guillermo Gonzalez (verterok) wrote : Posted in a previous version of this proposal

The changes looks good.
Could you add some tests checking the behavior of the UIFactory? Also, please could you add tests for at least the 2 cases with the new "auth" command?

Thanks.

review: Needs Fixing
Revision history for this message
Piotr Piastucki (piastucki) wrote :

The UIFactory tests are pretty straightforward, however, in order to test new authentication mechanism an FTP/SSH account is needed. I used a publicly available test account on a public FTP server - ftp.secureftp-test.com which has been up&running for a couple of year, but the test will stop working when it is shut down.

Revision history for this message
Guillermo Gonzalez (verterok) wrote :

Thanks, but we can't use an external service in the tests.

I'll mark this as approved and change the tests in a different branch.

Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: