Re: Reports as system-state Observers

[Christopher James Halse Rogers]

On Fri, Sep 16, 2016 at 4:51 PM, Alan Griffiths 
 wrote:

On 16/09/16 02:51, Christopher James Halse Rogers wrote:
On Thu, Sep 15, 2016 at 12:11 PM, Daniel van Vugt 
 wrote:
So actually... I now think it's OK providing the base class is 
named *Observer. And only some implementations would be called 
*Report.


I would also be happy with this; various components take an Observer 
(which provides a register-interested-party API), and Reports 
register themselves as interested parties.


1. the user of the "Report" interface is the core code - which is 
simply reporting something. The code reads "report->xxx()" which is 
clear.

2. we've had this name for years, without considering it a problem.

3. Some implementations of "Report" log, some don't - which is the 
behaviour that was originally intended (and what we all want).


4. I *think* the current issue is simply that we want to add support 
for multiple reports/observers so that code using Mir can get 
notifications without disabling the supplied logging/lttng options. 
We have existing generic "observer" code that can be used to 
composite reports.


In short, I agree that "Reports" are taking the "Observer" role from 
the pattern language, but I think the more specific name is good here 
and I don't follow why folks want the pain of a rename.


So, I think what's happening here is:

For some subset (including me) of the Mir team, the term “Report” 
has a bunch of implicit semantics, including

*) This is optional, for post-hoc human-based debugging only
*) This is *purely* observational; it might log in any number of ways, 
but the *only* state it's going to change is the state of some logging 
system.
*) The calls are approximately equal to printf; relatively cheap and 
simple.


Which is why (1) and (2) are problematic - we *do* think that the 
meaning of report->xxx() is clear, and isn't a problem, but we think it 
means something significantly different to what you do.



--
Mir-devel mailing list
Mir-devel@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/mir-devel

Re: Reports as system-state Observers

[Alan Griffiths]

On 16/09/16 08:01, Daniel van Vugt wrote:
> However the more accurate naming of the interface being an "Observer"

I guess this is the root of the discussion: I think "Report" is the more
precise name.


-- 
Mir-devel mailing list
Mir-devel@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/mir-devel

Re: Reports as system-state Observers

[Alan Griffiths]

On 16/09/16 02:51, Christopher James Halse Rogers wrote:
> On Thu, Sep 15, 2016 at 12:11 PM, Daniel van Vugt
>  wrote:
>> So actually... I now think it's OK providing the base class is named
>> *Observer. And only some implementations would be called *Report.
>
> I would also be happy with this; various components take an Observer
> (which provides a register-interested-party API), and Reports register
> themselves as interested parties. 

1. the user of the "Report" interface is the core code - which is simply
reporting something. The code reads "report->xxx()" which is clear.

2. we've had this name for years, without considering it a problem.

3. Some implementations of "Report" log, some don't - which is the
behaviour that was originally intended (and what we all want).

4. I *think* the current issue is simply that we want to add support for
multiple reports/observers so that code using Mir can get notifications
without disabling the supplied logging/lttng options. We have existing
generic "observer" code that can be used to composite reports.

In short, I agree that "Reports" are taking the "Observer" role from the
pattern language, but I think the more specific name is good here and I
don't follow why folks want the pain of a rename.

With regard to Chris's MP - I don't believe we want both a "Report" and
an "Observer" both doing the same job as a solution to /4/.
-- 
Mir-devel mailing list
Mir-devel@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/mir-devel

Re: Reports as system-state Observers

[Daniel van Vugt]

That's a good point. We don't need to change all the reporting locations.

I still don't like the idea of having to delegate within reports, as 
well as changing their construction methods.


Although I now realize my other concern that we'd have to change all 
reports at once for consistency also isn't true. If a user said:


  --something-report=log,lttng,...

and the SomethingReport didn't support multiple implementations yet then 
that's just a syntax error.


I guess my remaining concern that's unresolved is the existing 
assumption that reports don't affect the control flow of the server. But 
that's really just a naming issue -- if it was called SomethingObserver 
then it would be OK. Just the word "report" makes me think it is 
ineffectual to server behavior.



On 15/09/16 10:01, Christopher James Halse Rogers wrote:

On Thu, Sep 15, 2016 at 11:57 AM, Daniel van Vugt
 wrote:

Not impossible, but it feels too ugly. I would prefer to keep reports
for debugging/logging only where they don't affect the behavior of the
server. And programmers can assume this is true without knowing the
implementation details of any given subsystem.


I agree, but



The implementation is potentially also ugly. At each point where a
report happens you can't any more say:
   report_interface->report_something()
because that would only call one implementation. So we would need to
change the architecture of all reports and for consistency change them
all. It doesn't sound nice.


...this wouldn't actually be true? We'd just make the_report_interface()
not an override point and instead always have the same implementation
(which would then delegate to whichever report implementations have been
registered).




--
Mir-devel mailing list
Mir-devel@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/mir-devel