You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Mark Chu-Carroll <mc...@twopensource.com> on 2014/03/12 20:57:55 UTC
Review Request 19143: Catch errors thrown by authentication modules.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19143/
-----------------------------------------------------------
Review request for Aurora, Kevin Sweeney and Brian Wickman.
Bugs: aurora-259
https://issues.apache.org/jira/browse/aurora-259
Repository: aurora
Description
-------
Catch errors thrown by authentication modules.
Authentication modules can throw arbitrary errors - ldap exceptions,
password errors, etc, if auth fails with one of these app specific
errors, the client current dumps its cookies in a stack dump. This
change catches the error cleanly, and transforms it into a handled
authentication exception.
Diffs
-----
src/main/python/apache/aurora/client/api/scheduler_client.py f770df7a23779f919cd11cc28b2aaf7cfdf9c5a1
Diff: https://reviews.apache.org/r/19143/diff/
Testing
-------
Thanks,
Mark Chu-Carroll
Re: Review Request 19143: Catch errors thrown by authentication modules.
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19143/#review37409
-----------------------------------------------------------
Ship it!
Ship It!
- Kevin Sweeney
On March 12, 2014, 3:59 p.m., Mark Chu-Carroll wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19143/
> -----------------------------------------------------------
>
> (Updated March 12, 2014, 3:59 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
>
>
> Bugs: aurora-259
> https://issues.apache.org/jira/browse/aurora-259
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Catch errors thrown by authentication modules.
>
> Authentication modules can throw arbitrary errors - ldap exceptions,
> password errors, etc, if auth fails with one of these app specific
> errors, the client current dumps its cookies in a stack dump. This
> change catches the error cleanly, and transforms it into a handled
> authentication exception.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/scheduler_client.py f770df7a23779f919cd11cc28b2aaf7cfdf9c5a1
>
> Diff: https://reviews.apache.org/r/19143/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mark Chu-Carroll
>
>
Re: Review Request 19143: Catch errors thrown by authentication modules.
Posted by Brian Wickman <wi...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19143/#review37392
-----------------------------------------------------------
Ship it!
Ship It!
- Brian Wickman
On March 12, 2014, 10:59 p.m., Mark Chu-Carroll wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19143/
> -----------------------------------------------------------
>
> (Updated March 12, 2014, 10:59 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
>
>
> Bugs: aurora-259
> https://issues.apache.org/jira/browse/aurora-259
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Catch errors thrown by authentication modules.
>
> Authentication modules can throw arbitrary errors - ldap exceptions,
> password errors, etc, if auth fails with one of these app specific
> errors, the client current dumps its cookies in a stack dump. This
> change catches the error cleanly, and transforms it into a handled
> authentication exception.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/scheduler_client.py f770df7a23779f919cd11cc28b2aaf7cfdf9c5a1
>
> Diff: https://reviews.apache.org/r/19143/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mark Chu-Carroll
>
>
Re: Review Request 19143: Catch errors thrown by authentication modules.
Posted by Mark Chu-Carroll <mc...@twopensource.com>.
> On March 13, 2014, 2:26 p.m., Joshua Cohen wrote:
> > src/main/python/apache/aurora/client/api/scheduler_client.py, line 243
> > <https://reviews.apache.org/r/19143/diff/2/?file=517784#file517784line243>
> >
> > The broad catch below worries me a bit. Would it be feasible to have the various _connect impls (which presumably are what are raising the various types of exceptions and therefore have a better idea of what types of exceptions might be raised) be responsible for catching the appropriate exception types and re-raising as auth errors if necessary, or is there really no way of narrowing this down to only auth errors?
It would be difficult. The way that some of the auth code is patched in makes it very hard to control what exceptions get raised.
(I don't want to go into more detail here; discussing how twitter does authentication on the open-source jira isn't a good idea.)
- Mark
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19143/#review37080
-----------------------------------------------------------
On March 12, 2014, 6:59 p.m., Mark Chu-Carroll wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19143/
> -----------------------------------------------------------
>
> (Updated March 12, 2014, 6:59 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
>
>
> Bugs: aurora-259
> https://issues.apache.org/jira/browse/aurora-259
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Catch errors thrown by authentication modules.
>
> Authentication modules can throw arbitrary errors - ldap exceptions,
> password errors, etc, if auth fails with one of these app specific
> errors, the client current dumps its cookies in a stack dump. This
> change catches the error cleanly, and transforms it into a handled
> authentication exception.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/scheduler_client.py f770df7a23779f919cd11cc28b2aaf7cfdf9c5a1
>
> Diff: https://reviews.apache.org/r/19143/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mark Chu-Carroll
>
>
Re: Review Request 19143: Catch errors thrown by authentication modules.
Posted by Joshua Cohen <jc...@twitter.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19143/#review37080
-----------------------------------------------------------
src/main/python/apache/aurora/client/api/scheduler_client.py
<https://reviews.apache.org/r/19143/#comment68367>
The broad catch below worries me a bit. Would it be feasible to have the various _connect impls (which presumably are what are raising the various types of exceptions and therefore have a better idea of what types of exceptions might be raised) be responsible for catching the appropriate exception types and re-raising as auth errors if necessary, or is there really no way of narrowing this down to only auth errors?
- Joshua Cohen
On March 12, 2014, 10:59 p.m., Mark Chu-Carroll wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19143/
> -----------------------------------------------------------
>
> (Updated March 12, 2014, 10:59 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
>
>
> Bugs: aurora-259
> https://issues.apache.org/jira/browse/aurora-259
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Catch errors thrown by authentication modules.
>
> Authentication modules can throw arbitrary errors - ldap exceptions,
> password errors, etc, if auth fails with one of these app specific
> errors, the client current dumps its cookies in a stack dump. This
> change catches the error cleanly, and transforms it into a handled
> authentication exception.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/scheduler_client.py f770df7a23779f919cd11cc28b2aaf7cfdf9c5a1
>
> Diff: https://reviews.apache.org/r/19143/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mark Chu-Carroll
>
>
Re: Review Request 19143: Catch errors thrown by authentication modules.
Posted by Mark Chu-Carroll <mc...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19143/
-----------------------------------------------------------
(Updated March 12, 2014, 6:59 p.m.)
Review request for Aurora, Kevin Sweeney and Brian Wickman.
Changes
-------
Added debug logging of stack traceback for auth errors.
Bugs: aurora-259
https://issues.apache.org/jira/browse/aurora-259
Repository: aurora
Description
-------
Catch errors thrown by authentication modules.
Authentication modules can throw arbitrary errors - ldap exceptions,
password errors, etc, if auth fails with one of these app specific
errors, the client current dumps its cookies in a stack dump. This
change catches the error cleanly, and transforms it into a handled
authentication exception.
Diffs (updated)
-----
src/main/python/apache/aurora/client/api/scheduler_client.py f770df7a23779f919cd11cc28b2aaf7cfdf9c5a1
Diff: https://reviews.apache.org/r/19143/diff/
Testing
-------
Thanks,
Mark Chu-Carroll
Re: Review Request 19143: Catch errors thrown by authentication modules.
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19143/#review36996
-----------------------------------------------------------
src/main/python/apache/aurora/client/api/scheduler_client.py
<https://reviews.apache.org/r/19143/#comment68245>
I'm pretty wary of swallowing exceptions like this - seems like an easy way to paper over bugs / hide truly exceptional events. I'd at least log the traceback at debug level here so that it's available in debug output.
- Kevin Sweeney
On March 12, 2014, 12:57 p.m., Mark Chu-Carroll wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19143/
> -----------------------------------------------------------
>
> (Updated March 12, 2014, 12:57 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
>
>
> Bugs: aurora-259
> https://issues.apache.org/jira/browse/aurora-259
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Catch errors thrown by authentication modules.
>
> Authentication modules can throw arbitrary errors - ldap exceptions,
> password errors, etc, if auth fails with one of these app specific
> errors, the client current dumps its cookies in a stack dump. This
> change catches the error cleanly, and transforms it into a handled
> authentication exception.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/scheduler_client.py f770df7a23779f919cd11cc28b2aaf7cfdf9c5a1
>
> Diff: https://reviews.apache.org/r/19143/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mark Chu-Carroll
>
>