inasafe / inasafe

InaSAFE - QGIS plugin for estimating impact from natural disasters
www.inasafe.org
GNU General Public License v3.0
256 stars 136 forks source link

RFC InaSAFE Messaging Dispatcher #577

Closed timlinux closed 10 years ago

timlinux commented 11 years ago

Problem

This ticket describes a proposed solution for these interrelated problems.

Detail

In our work to resolve issues in #574 it has become apparent that we need a better solution for passing messages around in InaSAFE. With the current (pre 1.2) state of code, dock is a monolithic class that instantiates numerous objects and displays messages to the user based on state changes in those objects.

With that implementation it is not really possible for e.g. an impact function to notify a user about its progress.

In some cases (e.g. aggregation and post processing code), the code relies on the fact that it is embedded in the Dock class to relay messages to the user, which results in the functionalities of Dock, aggregation code and post processing code to be tightly coupled. Any attempt to decouple these functionalities results in the need for the implementation of callbacks or state interrogator logic.

This tight coupling also makes it difficult to write unit tests in a clean, modular way. With the status quo, e.g. aggregator tests require a dock instance, both to receive state information (e.g. which layers are available) and receive display state changes (e.g. showing a message in the dock which a unit test verifies). Our work in #574 decouples the need for aggregator and dock to be comingled in order for e.g. aggregator to know which layer is to be used as a hazard layer. The work proposed in this issue will decouple the message passing aspects such that e.g. a unit test could provide its on Message handler class and verify the aggregator behaved correctly without ever needing a dock instance to exist.

We believe that any part of the InaSAFE code base should be able to send a message to any message display platform (for instance a log file, the dock web widget) in a loosely coupled way.

Proposal

Our proposal is to implement 3 new functional elements:

safemessagingarchitecture2 1

Messaging architecture (used to transport messages)

safemessagingarchitecture 3

safemessagingarchitecture 1

safemessagingarchitecture

The dispatcher mechanism is documented here: http://pydispatcher.sourceforge.net/pydoc/pydispatch.dispatcher.html

ErrorMessage Architecture (used to represent and propogate error messages)

inasafeerrormessagearchitecture

Implementation

@ingenieroariel @vanpuk @ismailsunni @mbernasocchi @mach0 @uniomni

timlinux commented 11 years ago

Some screenshots showing the implemented functionality

Initial message shown on application start

selection_001

Message display including an ErrorMessage

(Note: The following screenshots are all one continous message but I needed to chop it up into smaller screenshots in order to capture it)

selection_002 selection_003 selection_004 selection_005

Message display including a successful analysis completion

(Note: The following screenshots are all one continous message but I needed to chop it up into smaller screenshots in order to capture it)

selection_006 selection_007

selection_008

vanpuk commented 11 years ago

Love the break down, But can I just confirm if there is a problem you will need to scroll down to see it, or will the red banner come to the top of the InaSAFE window?

Few points to change in "gettiong started": dwellings replace with structure (as this is the key word used)

  1. Neither BNPB, AusAID, WorldBank take any responsibility............. Its up to the GFDRR guys if they want to specifically mention GFDRR in this statement. But for us, by saying AusAID it covers AIFDR as well.
timlinux commented 11 years ago

Hi

But can I just confirm if there is a problem you will need to scroll down to see it, or will the red banner come to the top of the InaSAFE window?

This is on our TODO list, the dock will scroll to the messages as they are added.

Few points to change in "gettiong started": dwellings replace with structure (as this is the key word used)

Ok done!

  1. Neither BNPB, AusAID, WorldBank take any responsibility............. Its up to the GFDRR guys if they want to specifically mention GFDRR in this statement. But for us, by saying AusAID it covers AIFDR as well.

Yes Abby sent me a message while back specifically requesting this. I have updated the wording as follows:

'Neither BNPB, AusAID, nor the World Bank-GFDRR, take any ' 'responsibility for the correctness of outputs from InaSAFE ' 'decisions derived as a consequence.'

ingenieroariel commented 11 years ago

@timlinux thanks for this proposal.

Here are some questions:

  1. Why are we adding presentation logic to the message format? In my use case for inasafe, I prefer to be able to modify how the report looks like for different impact functions. A grammar based on inasafe calculations (success, error message, data tables, recommendations, calculation time) would be more useful than one based on LineBreak or EmphasizedText.
  2. In Django, there is already a comprehensive signal framework that could be used if needed, however, to effectively use it in the dock I see the need for websockets or something similar, the message part is less important as information about the calculation is written to the database anyway. Would this message dispatcher have to be on even when using safe as a library and not an application? BTW, here is a simple example of a django view calling an inasafe calculation(could be useful for context): https://github.com/ingenieroariel/webandgis/blob/master/layers/views.py#L43
  3. Serialization, do you foresee being possible to store the messages somewhere (in geonode safe there is a database backend) and then being able to print them / show them again?. The example is a calculation that produces a geospatial layer and an end user who wants to read the calculation output along with the other metadata.

Despite my concerns I think it is a very good improvement and a solution to the reporting problems from within QGIS.

Thanks, Ariel

timlinux commented 11 years ago

Hi Ariel

Thanks for the great feedback. Responses inline below!

Why are we adding presentation logic to the message format? In my use case for inasafe, I prefer to be able to modify how the report looks like for different impact functions. A > grammar based on inasafe calculations (success, error message, data tables, recommendations, calculation time) > would be more useful than one based on LineBreak or EmphasizedText.

We have both in place with the current implementation, but I would welcome further suggestions on how you would see further removal of presentation logic. Ultimately messages do need to be presented to the user and we should balance convenience with purity of implementation. The current implementation is very convenient and each element can be passed style information (e.g. class lists) if the user wishes to do so. The actual presentation of those styles is handled at the presentation layer and the message classes only produce markup. It might be easier to chat about this on a skype call and we can certainly evolve what we have in place in future versions to better align the implementation to your ideas.

In Django, there is already a comprehensive signal framework that could be used if needed, however, to effectively > use it in the dock I see the need for websockets or something similar, the message part is less important as information about the calculation is written to the database anyway. Would this message dispatcher have to be on even when using safe as a library and not an application? BTW, here is a simple example of a django view calling an inasafe calculation(could be useful for context): https://github.com/ingenieroariel/webandgis/blob/master/layers/views.py#L43

Then you will be pleasantly surprised to know that the dispatcher implementation we are using is exactly the same one that made its way into django! :-)

To quote from the web site at http://pydispatcher.sourceforge.net/ :

"PyDispatcher provides the Python programmer with a multiple-producer-multiple-consumer signal-registration and routing infrastructure for use in multiple contexts. The mechanism of PyDispatcher started life as a highly rated recipe in the Python Cookbook. The project aims to include various enhancements to the recipe developed during use in various applications. It is primarily maintained by Mike Fletcher. A derivative of the project provides the Django web framework's "signal" system."

I only added some predefined messaging semantics (static, dynamic and error messages) so that the presentation layer can (if it wants to) implement standard behaviours based on what type of signal was emitted.

Serialization, do you foresee being possible to store the messages somewhere (in geonode safe there is a database backend) and then being able to print them / show them again?. The example is a calculation that produces a geospatial layer and an end user who wants to read the calculation output along with the other metadata.

Yeah that is exactly where we are going with this - the idea is that each impact layer will have associated with it a .log (or similarly named) file which contains a serialised copy of all the messages which were generated during the calculation process. My plan is for 1.3 to implement a json serialiser for this (alternatively we could pickle it), I also would like in 1.3 to go through each impact function and let it emit detailed explanatory text to the user explaining what and how things are being carried out - this is to remove the 'black box' experience (though the information must be printed in a human and logical way).

timlinux commented 11 years ago

Just an update :