You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrei Sekretenko <as...@mesosphere.io> on 2019/06/25 16:28:02 UTC
Review Request 70945: Added a constructor with a list of suppressed
roles to Java V0 bindings.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70945/
-----------------------------------------------------------
Review request for mesos and Benjamin Mahler.
Bugs: MESOS-9849
https://issues.apache.org/jira/browse/MESOS-9849
Repository: mesos
Description
-------
Added a constructor with a list of suppressed roles to Java V0 bindings.
Diffs
-----
src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp a21aca23cbef27b3c54bf1ae5834cbb457608130
src/java/src/org/apache/mesos/MesosSchedulerDriver.java 55ebc8772d9183286c908b4dba342109f28394f4
Diff: https://reviews.apache.org/r/70945/diff/1/
Testing
-------
Thanks,
Andrei Sekretenko
Re: Review Request 70945: Added constructors with a list of
suppressed roles to Java V0 bindings.
Posted by Benjamin Mahler <bm...@apache.org>.
> On June 25, 2019, 7:30 p.m., Benjamin Mahler wrote:
> > src/java/src/org/apache/mesos/MesosSchedulerDriver.java
> > Lines 260-265 (patched)
> > <https://reviews.apache.org/r/70945/diff/1/?file=2152164#file2152164line260>
> >
> > Why only this overload? What if the caller doesn't have a credential to pass? E.g. `implicitAcknowledgements` was added to both credential and non-credential constructors.
> >
> > (we should really have a builder style api here)
>
> Andrei Sekretenko wrote:
> Added a constructor with credential.
>
> Regarding the builder interface... how do you estimate the risk that V0 frameworks will still be around when one more parameter needed for authenticating or subscribing will be introduced into Mesos? 6 constructors is alreay too much, 8 will be a nightmare.
That's ultimately a question for marathon folks, I can't predict when they will move to v1 (most other actively developed schedulers I'm aware of have moved).
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70945/#review216127
-----------------------------------------------------------
On July 1, 2019, 1:38 p.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70945/
> -----------------------------------------------------------
>
> (Updated July 1, 2019, 1:38 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-9849
> https://issues.apache.org/jira/browse/MESOS-9849
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added constructors with a list of suppressed roles to Java V0 bindings.
>
>
> Diffs
> -----
>
> src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp a21aca23cbef27b3c54bf1ae5834cbb457608130
> src/java/src/org/apache/mesos/MesosSchedulerDriver.java 55ebc8772d9183286c908b4dba342109f28394f4
>
>
> Diff: https://reviews.apache.org/r/70945/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 70945: Added constructors with a list of
suppressed roles to Java V0 bindings.
Posted by Andrei Sekretenko <as...@mesosphere.io>.
> On June 25, 2019, 7:30 p.m., Benjamin Mahler wrote:
> > src/java/src/org/apache/mesos/MesosSchedulerDriver.java
> > Lines 260-265 (patched)
> > <https://reviews.apache.org/r/70945/diff/1/?file=2152164#file2152164line260>
> >
> > Why only this overload? What if the caller doesn't have a credential to pass? E.g. `implicitAcknowledgements` was added to both credential and non-credential constructors.
> >
> > (we should really have a builder style api here)
Added a constructor with credential.
Regarding the builder interface... how do you estimate the risk that V0 frameworks will still be around when one more parameter needed for authenticating or subscribing will be introduced into Mesos? 6 constructors is alreay too much, 8 will be a nightmare.
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70945/#review216127
-----------------------------------------------------------
On July 1, 2019, 1:38 p.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70945/
> -----------------------------------------------------------
>
> (Updated July 1, 2019, 1:38 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-9849
> https://issues.apache.org/jira/browse/MESOS-9849
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added constructors with a list of suppressed roles to Java V0 bindings.
>
>
> Diffs
> -----
>
> src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp a21aca23cbef27b3c54bf1ae5834cbb457608130
> src/java/src/org/apache/mesos/MesosSchedulerDriver.java 55ebc8772d9183286c908b4dba342109f28394f4
>
>
> Diff: https://reviews.apache.org/r/70945/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 70945: Added a constructor with a list of
suppressed roles to Java V0 bindings.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70945/#review216127
-----------------------------------------------------------
src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp
Lines 553-554 (patched)
<https://reviews.apache.org/r/70945/#comment303135>
Let's be specific:
To be backwards compatible the lack of the field means we use an empty set, as before.
src/java/src/org/apache/mesos/MesosSchedulerDriver.java
Lines 260-265 (patched)
<https://reviews.apache.org/r/70945/#comment303139>
Why only this overload? What if the caller doesn't have a credential to pass? E.g. `implicitAcknowledgements` was added to both credential and non-credential constructors.
(we should really have a builder style api here)
- Benjamin Mahler
On June 25, 2019, 4:28 p.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70945/
> -----------------------------------------------------------
>
> (Updated June 25, 2019, 4:28 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-9849
> https://issues.apache.org/jira/browse/MESOS-9849
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added a constructor with a list of suppressed roles to Java V0 bindings.
>
>
> Diffs
> -----
>
> src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp a21aca23cbef27b3c54bf1ae5834cbb457608130
> src/java/src/org/apache/mesos/MesosSchedulerDriver.java 55ebc8772d9183286c908b4dba342109f28394f4
>
>
> Diff: https://reviews.apache.org/r/70945/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 70945: Added constructors with a list of
suppressed roles to Java V0 bindings.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70945/#review216301
-----------------------------------------------------------
Ship it!
Ship It!
- Benjamin Mahler
On July 1, 2019, 1:38 p.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70945/
> -----------------------------------------------------------
>
> (Updated July 1, 2019, 1:38 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-9849
> https://issues.apache.org/jira/browse/MESOS-9849
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added constructors with a list of suppressed roles to Java V0 bindings.
>
>
> Diffs
> -----
>
> src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp a21aca23cbef27b3c54bf1ae5834cbb457608130
> src/java/src/org/apache/mesos/MesosSchedulerDriver.java 55ebc8772d9183286c908b4dba342109f28394f4
>
>
> Diff: https://reviews.apache.org/r/70945/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 70945: Added constructors with a list of
suppressed roles to Java V0 bindings.
Posted by Andrei Sekretenko <as...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70945/
-----------------------------------------------------------
(Updated July 1, 2019, 1:38 p.m.)
Review request for mesos and Benjamin Mahler.
Summary (updated)
-----------------
Added constructors with a list of suppressed roles to Java V0 bindings.
Bugs: MESOS-9849
https://issues.apache.org/jira/browse/MESOS-9849
Repository: mesos
Description (updated)
-------
Added constructors with a list of suppressed roles to Java V0 bindings.
Diffs (updated)
-----
src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp a21aca23cbef27b3c54bf1ae5834cbb457608130
src/java/src/org/apache/mesos/MesosSchedulerDriver.java 55ebc8772d9183286c908b4dba342109f28394f4
Diff: https://reviews.apache.org/r/70945/diff/2/
Changes: https://reviews.apache.org/r/70945/diff/1-2/
Testing
-------
Thanks,
Andrei Sekretenko