You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2013/10/03 00:46:15 UTC
Review Request 14453: Improved MesosSchedulerDriver initialize() method to
properly handle authentication credentials.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14453/
-----------------------------------------------------------
Review request for mesos.
Repository: mesos-git
Description
-------
See summary.
Diffs
-----
src/java/jni/convert.hpp 51a93cacf2e8e98f972bda88d8be18f3ad31705e
src/java/jni/convert.cpp b76a5960cc9bd6828886083fb2793482a2b13c3a
src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 6d2a03b6a88e71ac4e2e2d1ee8e15925e393ef3d
src/java/src/org/apache/mesos/MesosSchedulerDriver.java 7ef1fe7755286bf92b94d7ece4f72d54e5b57a84
Diff: https://reviews.apache.org/r/14453/diff/
Testing
-------
make check
Tested with all different configurations for jar.
--> old jar which doesn't have credentials field
--> new jar which sets credentials to null
--> new jar which sets credentials
Thanks,
Vinod Kone
Re: Review Request 14453: Improved MesosSchedulerDriver initialize() method
to properly handle authentication credentials.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14453/
-----------------------------------------------------------
(Updated Oct. 8, 2013, 11:27 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
benh's. NNFR.
Bugs: MESOS-704
https://issues.apache.org/jira/browse/MESOS-704
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
src/java/jni/convert.hpp 51a93cacf2e8e98f972bda88d8be18f3ad31705e
src/java/jni/convert.cpp b76a5960cc9bd6828886083fb2793482a2b13c3a
src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 6d2a03b6a88e71ac4e2e2d1ee8e15925e393ef3d
src/java/src/org/apache/mesos/MesosSchedulerDriver.java 7ef1fe7755286bf92b94d7ece4f72d54e5b57a84
Diff: https://reviews.apache.org/r/14453/diff/
Testing
-------
make check
Tested with all different configurations for jar.
--> old jar which doesn't have credentials field
--> new jar which sets credentials to null
--> new jar which sets credentials
Thanks,
Vinod Kone
Re: Review Request 14453: Improved MesosSchedulerDriver initialize() method
to properly handle authentication credentials.
Posted by Vinod Kone <vi...@gmail.com>.
> On Oct. 7, 2013, 8:17 p.m., Benjamin Hindman wrote:
> > src/java/jni/convert.cpp, line 530
> > <https://reviews.apache.org/r/14453/diff/2/?file=361978#file361978line530>
> >
> > Can we do this once at the beginning of the function?
> >
> > static jclass NO_SUCH_FIELD_ERROR = NULL;
> >
> > if (NO_SUCH_FIELD_ERROR == NULL) {
> > NO_SUCH_FIELD_ERROR = env->FindClass("java/lang/NoSuchFieldError");
> > if (env->ExceptionOccured()) {
> > return Error("Cannot find class NoSuchFieldError");
> > }
> > }
> >
> > jfieldID field = env->GetFieldID(clazz, name, signature);
> > jthrowable jexception = env->ExceptionOccurred();
> > if (jexception != NULL) {
> > env->ExceptionClear(); // Clear the exception first before proceeding.
> > if (!env->IsInstanceOf(jexception, NO_SUCH_FIELD_ERROR)) {
> > // We are here if we got a different exception than
> > // 'NoSuchFieldError'. Rethrow and bail.
> > env->Throw(jexception);
> > return Error("Unexpected exception");
> > }
> > return None(); // Field doesn't exist.
> > }
> >
> > return field;
thanks!
> On Oct. 7, 2013, 8:17 p.m., Benjamin Hindman wrote:
> > src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp, line 568
> > <https://reviews.apache.org/r/14453/diff/2/?file=361979#file361979line568>
> >
> > if (jcredential != NULL) {
> >
> > Then we can only use one variable and I don't have to reason about whether or not it's possible to pass NULL to construct.
good point. fixed.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14453/#review26750
-----------------------------------------------------------
On Oct. 8, 2013, 11:27 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14453/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2013, 11:27 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Bugs: MESOS-704
> https://issues.apache.org/jira/browse/MESOS-704
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/java/jni/convert.hpp 51a93cacf2e8e98f972bda88d8be18f3ad31705e
> src/java/jni/convert.cpp b76a5960cc9bd6828886083fb2793482a2b13c3a
> src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 6d2a03b6a88e71ac4e2e2d1ee8e15925e393ef3d
> src/java/src/org/apache/mesos/MesosSchedulerDriver.java 7ef1fe7755286bf92b94d7ece4f72d54e5b57a84
>
> Diff: https://reviews.apache.org/r/14453/diff/
>
>
> Testing
> -------
>
> make check
>
> Tested with all different configurations for jar.
> --> old jar which doesn't have credentials field
> --> new jar which sets credentials to null
> --> new jar which sets credentials
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 14453: Improved MesosSchedulerDriver initialize() method
to properly handle authentication credentials.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14453/#review26750
-----------------------------------------------------------
Ship it!
src/java/jni/convert.cpp
<https://reviews.apache.org/r/14453/#comment52068>
!= NULL
src/java/jni/convert.cpp
<https://reviews.apache.org/r/14453/#comment52067>
Spacing issues.
src/java/jni/convert.cpp
<https://reviews.apache.org/r/14453/#comment52070>
Can we do this once at the beginning of the function?
static jclass NO_SUCH_FIELD_ERROR = NULL;
if (NO_SUCH_FIELD_ERROR == NULL) {
NO_SUCH_FIELD_ERROR = env->FindClass("java/lang/NoSuchFieldError");
if (env->ExceptionOccured()) {
return Error("Cannot find class NoSuchFieldError");
}
}
jfieldID field = env->GetFieldID(clazz, name, signature);
jthrowable jexception = env->ExceptionOccurred();
if (jexception != NULL) {
env->ExceptionClear(); // Clear the exception first before proceeding.
if (!env->IsInstanceOf(jexception, NO_SUCH_FIELD_ERROR)) {
// We are here if we got a different exception than
// 'NoSuchFieldError'. Rethrow and bail.
env->Throw(jexception);
return Error("Unexpected exception");
}
return None(); // Field doesn't exist.
}
return field;
src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp
<https://reviews.apache.org/r/14453/#comment52071>
Don't worry about re-throwing, it's already thrown, just return.
src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp
<https://reviews.apache.org/r/14453/#comment52072>
if (jcredential != NULL) {
Then we can only use one variable and I don't have to reason about whether or not it's possible to pass NULL to construct.
- Benjamin Hindman
On Oct. 7, 2013, 8:05 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14453/
> -----------------------------------------------------------
>
> (Updated Oct. 7, 2013, 8:05 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Bugs: MESOS-704
> https://issues.apache.org/jira/browse/MESOS-704
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/java/jni/convert.hpp 51a93cacf2e8e98f972bda88d8be18f3ad31705e
> src/java/jni/convert.cpp b76a5960cc9bd6828886083fb2793482a2b13c3a
> src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 6d2a03b6a88e71ac4e2e2d1ee8e15925e393ef3d
> src/java/src/org/apache/mesos/MesosSchedulerDriver.java 7ef1fe7755286bf92b94d7ece4f72d54e5b57a84
>
> Diff: https://reviews.apache.org/r/14453/diff/
>
>
> Testing
> -------
>
> make check
>
> Tested with all different configurations for jar.
> --> old jar which doesn't have credentials field
> --> new jar which sets credentials to null
> --> new jar which sets credentials
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 14453: Improved MesosSchedulerDriver initialize() method
to properly handle authentication credentials.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14453/
-----------------------------------------------------------
(Updated Oct. 7, 2013, 8:05 a.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
rebased.
Bugs: MESOS-704
https://issues.apache.org/jira/browse/MESOS-704
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
src/java/jni/convert.hpp 51a93cacf2e8e98f972bda88d8be18f3ad31705e
src/java/jni/convert.cpp b76a5960cc9bd6828886083fb2793482a2b13c3a
src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 6d2a03b6a88e71ac4e2e2d1ee8e15925e393ef3d
src/java/src/org/apache/mesos/MesosSchedulerDriver.java 7ef1fe7755286bf92b94d7ece4f72d54e5b57a84
Diff: https://reviews.apache.org/r/14453/diff/
Testing
-------
make check
Tested with all different configurations for jar.
--> old jar which doesn't have credentials field
--> new jar which sets credentials to null
--> new jar which sets credentials
Thanks,
Vinod Kone