You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@mesos.apache.org by Alexander Rojas <al...@mesosphere.io> on 2018/06/14 10:10:41 UTC
Should we remove `noexcept` from `ObjectApprover::approved()`
signature?
I may have brought up this issue in the past, however I’m bringing it again, The `ObjectApprover::approved()` [1] method has the following signature:
```
virtual Try<bool> approved(
const Option<Object>& object) const noexcept = 0;
```
This is unfortunate since it is impossible to mock a function in google mock with two qualifiers [2] without some modifications to gmock itself. this reduces the amount of tests we can perform.
Moreover, the `noexcept` qualifier is not even needed in Mesos, since it does not use exceptions and it is compiled without exception support by default.
The tricky situation here is that this is a public API so it would be tricky to replace since it will break backwards compatibility. So I’m calling out to any modules developer to notify if they are ok with the change or if we should instead try to modify gmock.
[1] https://github.com/apache/mesos/blob/8b93fa3/include/mesos/authorizer/authorizer.hpp#L221
[2] https://groups.google.com/forum/#!topic/googlemock/LsbubY26qx4
Alexander Rojas
alexander@mesosphere.io
Re: Should we remove `noexcept` from `ObjectApprover::approved()`
signature?
Posted by Benjamin Bannier <be...@mesosphere.io>.
Hi,
I still believe that declaring methods of this module interface `except` is a good thing which IMO we should also do for all new module interfaces going forward. We do not perform any exception handling around calls to these functions in Mesos, and `noexcept` is intended to communicate exactly that.
Googletest (which has in the meantime absorbed googlemock) is preparing their 1.9.0 release with C++11 support, and it is expected to be released “soon”, so I am not sure that breaking backwards compatibility to end up with weaker interfaces will be that worthwhile in the long run. I left a sketch for a possible workaround in the issue you created, https://issues.apache.org/jira/browse/MESOS-8991?focusedCommentId=16512313.
Let’s continue this discussion on the dev mailing list.
Cheers,
Benjamin
> On Jun 14, 2018, at 12:47 PM, Benno Evers <be...@mesosphere.com> wrote:
>
> Hi Alexander,
>
> > and it is compiled without exception support by default.
>
> What exactly do you mean by "without support"? My local libmesos.so includes 500 KiB of unwind tables, and we had issues like MESOS-8417 that are caused by unexpected exceptions being thrown.
>
> On Thu, Jun 14, 2018 at 12:10 PM, Alexander Rojas <al...@mesosphere.io> wrote:
> I may have brought up this issue in the past, however I’m bringing it again, The `ObjectApprover::approved()` [1] method has the following signature:
>
> ```
> virtual Try<bool> approved(
> const Option<Object>& object) const noexcept = 0;
> ```
>
> This is unfortunate since it is impossible to mock a function in google mock with two qualifiers [2] without some modifications to gmock itself. this reduces the amount of tests we can perform.
>
> Moreover, the `noexcept` qualifier is not even needed in Mesos, since it does not use exceptions and it is compiled without exception support by default.
>
> The tricky situation here is that this is a public API so it would be tricky to replace since it will break backwards compatibility. So I’m calling out to any modules developer to notify if they are ok with the change or if we should instead try to modify gmock.
>
> [1] https://github.com/apache/mesos/blob/8b93fa3/include/mesos/authorizer/authorizer.hpp#L221
> [2] https://groups.google.com/forum/#!topic/googlemock/LsbubY26qx4
>
>
>
> Alexander Rojas
> alexander@mesosphere.io
>
>
>
>
>
>
>
> --
> Benno Evers
> Software Engineer, Mesosphere
Re: Should we remove `noexcept` from `ObjectApprover::approved()`
signature?
Posted by Benjamin Bannier <be...@mesosphere.io>.
Hi,
I still believe that declaring methods of this module interface `except` is a good thing which IMO we should also do for all new module interfaces going forward. We do not perform any exception handling around calls to these functions in Mesos, and `noexcept` is intended to communicate exactly that.
Googletest (which has in the meantime absorbed googlemock) is preparing their 1.9.0 release with C++11 support, and it is expected to be released “soon”, so I am not sure that breaking backwards compatibility to end up with weaker interfaces will be that worthwhile in the long run. I left a sketch for a possible workaround in the issue you created, https://issues.apache.org/jira/browse/MESOS-8991?focusedCommentId=16512313.
Let’s continue this discussion on the dev mailing list.
Cheers,
Benjamin
> On Jun 14, 2018, at 12:47 PM, Benno Evers <be...@mesosphere.com> wrote:
>
> Hi Alexander,
>
> > and it is compiled without exception support by default.
>
> What exactly do you mean by "without support"? My local libmesos.so includes 500 KiB of unwind tables, and we had issues like MESOS-8417 that are caused by unexpected exceptions being thrown.
>
> On Thu, Jun 14, 2018 at 12:10 PM, Alexander Rojas <al...@mesosphere.io> wrote:
> I may have brought up this issue in the past, however I’m bringing it again, The `ObjectApprover::approved()` [1] method has the following signature:
>
> ```
> virtual Try<bool> approved(
> const Option<Object>& object) const noexcept = 0;
> ```
>
> This is unfortunate since it is impossible to mock a function in google mock with two qualifiers [2] without some modifications to gmock itself. this reduces the amount of tests we can perform.
>
> Moreover, the `noexcept` qualifier is not even needed in Mesos, since it does not use exceptions and it is compiled without exception support by default.
>
> The tricky situation here is that this is a public API so it would be tricky to replace since it will break backwards compatibility. So I’m calling out to any modules developer to notify if they are ok with the change or if we should instead try to modify gmock.
>
> [1] https://github.com/apache/mesos/blob/8b93fa3/include/mesos/authorizer/authorizer.hpp#L221
> [2] https://groups.google.com/forum/#!topic/googlemock/LsbubY26qx4
>
>
>
> Alexander Rojas
> alexander@mesosphere.io
>
>
>
>
>
>
>
> --
> Benno Evers
> Software Engineer, Mesosphere
Re: Should we remove `noexcept` from `ObjectApprover::approved()` signature?
Posted by Benno Evers <be...@mesosphere.com>.
Hi Alexander,
> and it is compiled without exception support by default.
What exactly do you mean by "without support"? My local libmesos.so
includes 500 KiB of unwind tables, and we had issues like MESOS-8417 that
are caused by unexpected exceptions being thrown.
On Thu, Jun 14, 2018 at 12:10 PM, Alexander Rojas <al...@mesosphere.io>
wrote:
> I may have brought up this issue in the past, however I’m bringing it
> again, The `ObjectApprover::approved()` [1] method has the following
> signature:
>
> ```
> virtual Try<bool> approved(
> const Option<Object>& object) const noexcept = 0;
> ```
>
> This is unfortunate since it is impossible to mock a function in google
> mock with two qualifiers [2] without some modifications to gmock itself.
> this reduces the amount of tests we can perform.
>
> Moreover, the `noexcept` qualifier is not even needed in Mesos, since it
> does not use exceptions and it is compiled without exception support by
> default.
>
> The tricky situation here is that this is a public API so it would be
> tricky to replace since it will break backwards compatibility. So I’m
> calling out to any modules developer to notify if they are ok with the
> change or if we should instead try to modify gmock.
>
> [1] https://github.com/apache/mesos/blob/8b93fa3/include/
> mesos/authorizer/authorizer.hpp#L221
> [2] https://groups.google.com/forum/#!topic/googlemock/LsbubY26qx4
>
>
>
> Alexander Rojas
> alexander@mesosphere.io
>
>
>
>
>
--
Benno Evers
Software Engineer, Mesosphere