[WIP] Adding MessageBus

Review Request #9235 — Created Sept. 29, 2017 and discarded

Information

Review Board
master

Reviewers

On the client side message requests can be registered at a desired time
interval. The minimum desired time interval is the interval in which
message requests are made to MessageBus, at url /messagebus/. There is
currently no handling of the payload response. A MessageRequest called
ReviewRequestUpdatesMessageRequest was created to handle storing
relevant data (namely the review request id), which is sent alongside the
associated message_provider_id as parameters of the request to the
message bus.

On the server side, requests, identified by their message_provider_id,
pass their data to the appropriate MessageProvider, which is responsible
for accumulating messages and writing the data to the payload. Currently,
a message provider has been created for review request updates, ultimately
to replace the existing functionality of the ReviewRequestUpdatesView.
This MessageProvider returns the exact same data as you would expect by
going to the url /r/<#>/_updates.

A ReviewRequestUpdatesMessageRequest was created and registered to the
client side message bus. The request is received by the server side
message bus, and the request arguments are passed to the
ReviewRequestUpdatesMessageProvider. Any entries registered to the
message request have their payloads properly returned. Not providing an
entry will request all the entries. The data returned, which consists of
two payloads which match up exactly with what url /r/<#>/_updates would
output (message bus is going to replace this, so consistency between the
two is important).

Description From Last Updated

F401 'django.conf.urls.patterns' imported but unused

reviewbotreviewbot

F401 'reviewboard.messagebus.views' imported but unused

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

Col: 10 Expected an assignment or function call and instead saw an expression.

reviewbotreviewbot

Col: 35 Expected '===' and instead saw '=='.

reviewbotreviewbot

Col: 6 Missing semicolon.

reviewbotreviewbot

F401 'django.conf.urls.patterns' imported but unused

reviewbotreviewbot

F401 'reviewboard.messagebus.views' imported but unused

reviewbotreviewbot

F401 'django.template.loader' imported but unused

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

E703 statement ends with a semicolon

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

E231 missing whitespace after ':'

reviewbotreviewbot

Col: 29 Missing '()' invoking a constructor.

reviewbotreviewbot

Col: 14 Missing 'new' prefix when invoking a constructor.

reviewbotreviewbot

This file is missing a file-level docstring.

brenniebrennie

F401 'djblets.registries.registry.ALREADY_REGISTERED' imported but unused

reviewbotreviewbot

F401 'reviewboard.messagebus.provider.MessageProvider' imported but unused

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

E265 block comment should start with '# '

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

Missing a class-level docstring. This class should inheirit from object.

brenniebrennie

unicode.

brenniebrennie

Remove this blank line.

brenniebrennie

Blank line between these.

brenniebrennie

We should use six.iteritems(obj) because in Python 2, dict.items returns a list instead of a generator. You can get six …

brenniebrennie

Blank line between these.

brenniebrennie

Missing a docstring.

brenniebrennie

F821 undefined name 'payload'

reviewbotreviewbot

E128 continuation line under-indented for visual indent

reviewbotreviewbot

F401 'datetime' imported but unused

reviewbotreviewbot

E231 missing whitespace after ':'

reviewbotreviewbot

E127 continuation line over-indented for visual indent

reviewbotreviewbot

F841 local variable 'x' is assigned to but never used

reviewbotreviewbot

E303 too many blank lines (3)

reviewbotreviewbot

F841 local variable 'x' is assigned to but never used

reviewbotreviewbot

F401 'reviewboard.messagebus.views' imported but unused

reviewbotreviewbot

This list should be kept in alphabetical order.

brenniebrennie

Blank line between ) and {. Same for all cases below.

brenniebrennie

Col: 49 Expected '===' and instead saw '=='.

reviewbotreviewbot

Col: 22 Missing semicolon.

reviewbotreviewbot

Space after //. Missing period.

brenniebrennie

You can use an arrow function here, e.g. setTimeout(() => model._loadUpdates(), this._intervalMS);

brenniebrennie

Col: 10 'MBRequest' is defined but never used.

reviewbotreviewbot

This should be a member of the RB namespace e.g. RB.MessageBusRequest = function MessageBusRequest() {}; Also, this might be a …

brenniebrennie

Col: 19 Missing semicolon.

reviewbotreviewbot

Col: 25 Missing semicolon.

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

E265 block comment should start with '# '

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

F401 'django.contrib.auth.decorators.login_required' imported but unused

reviewbotreviewbot

F401 'django.utils.decorators.method_decorator' imported but unused

reviewbotreviewbot

F401 'django.utils.six.moves.cStringIO as StringIO' imported but unused

reviewbotreviewbot

F401 'reviewboard.reviews.views.ReviewRequestViewMixin' imported but unused

reviewbotreviewbot

F401 'reviewboard.site.decorators.check_local_site_access' imported but unused

reviewbotreviewbot

E128 continuation line under-indented for visual indent

reviewbotreviewbot

F821 undefined name 'dateutil'

reviewbotreviewbot

F821 undefined name 'is_aware'

reviewbotreviewbot

F821 undefined name 'make_aware'

reviewbotreviewbot

F821 undefined name 'utc'

reviewbotreviewbot

F821 undefined name 'HttpResponseBadRequest'

reviewbotreviewbot

F821 undefined name 'Http404'

reviewbotreviewbot

F821 undefined name 'SiteConfiguration'

reviewbotreviewbot

F821 undefined name 'get_last_line_number_in_diff'

reviewbotreviewbot

F821 undefined name 'get_file_chunks_in_range'

reviewbotreviewbot

F821 undefined name 'get_last_header_before_line'

reviewbotreviewbot

F821 undefined name 'Site'

reviewbotreviewbot

F821 undefined name 'exception_traceback_string'

reviewbotreviewbot

F821 undefined name 'Site'

reviewbotreviewbot

Col: 25 ['review_id'] is better written in dot notation.

reviewbotreviewbot

E501 line too long (80 > 79 characters)

reviewbotreviewbot

E127 continuation line over-indented for visual indent

reviewbotreviewbot

E127 continuation line over-indented for visual indent

reviewbotreviewbot

MBRequest doesn't seem to be a thing (here and elsewhere). Do you mean RB.MessageRequest?

brenniebrennie

Col: 39 Missing semicolon.

reviewbotreviewbot

F401 'django.utils.six' imported but unused

reviewbotreviewbot

E128 continuation line under-indented for visual indent

reviewbotreviewbot

E501 line too long (81 > 79 characters)

reviewbotreviewbot

E265 block comment should start with '# '

reviewbotreviewbot

E501 line too long (81 > 79 characters)

reviewbotreviewbot

E501 line too long (88 > 79 characters)

reviewbotreviewbot

E303 too many blank lines (2)

reviewbotreviewbot

F401 'django.http.Http404' imported but unused

reviewbotreviewbot

E127 continuation line over-indented for visual indent

reviewbotreviewbot

E265 block comment should start with '# '

reviewbotreviewbot

Col: 28 Missing semicolon.

reviewbotreviewbot

E127 continuation line over-indented for visual indent

reviewbotreviewbot

E128 continuation line under-indented for visual indent

reviewbotreviewbot
NI
Review request changed

Checks run (2 failed)

flake8 failed.
JSHint failed.

flake8

JSHint

NI
Review request changed

Change Summary:

Changed how MessageBus works, and added more functionality.

Summary:

-[WIP] Adding MessageBus as an app.
+[WIP] Adding MessageBus

Description:

~  

Created the endpoint for MessageBus. Currently, it only shows "Page Test" as the page message (I had to learn how Django URLs worked). The idea will be to use this address to send the aggregated request data (and on the python side, process it). Currently none of that is implemented.

  ~

On the client side message requests can be registered at a desired time

  + interval. The minimum desired time interval is the interval in which
  + message requests are made to MessageBus, at url /messagebus/. Currently,
  + registering requests does not have an effect on the data being returned
  + from the server (a test request is made instead).

  +
  +

On the server side, MessageProviders can be registered through MessageBus

  + to a registry. MessageBus requests will seperate individual message
  + requests, and pass the corresponding request to their appropriate
  + MessageProvider, if it exists (currently, there is no error handling if the
  + MessageProvider doesn't exist). The MessageProvider handles getting any
  + data, and returns it to MessageBus to be aggregrated and sent back to the
  + client.

Testing Done:

~  

Going to the address /messagebus/ loads without error, and properly displays "Page Test".

  ~

Created a test MessageProvider and registered it to MessageBus. There is

  + some work done on registering message requests, but it isn't currently in
  + use. A test request is made to the MessageBus, and the request is passed to
  + the test MessageProvider. Different MessageProviders (and message requests)
  + can be made. The test request is currently only processed to return a
  + string, but in practice could be used to query the web api.

Commit:

-8dff877cd03fc6abed6ffb7754764f3fc3084ad7
+1a2f0e9bf6905ace4096a5a67cfab0d383881eb3

Diff:

Revision 3 (+42)

Show changes

Checks run (2 failed)

flake8 failed.
JSHint failed.

flake8

JSHint

NI
Review request changed

Change Summary:

Added the ability to register callbacks for message requests, and changed the payload format.

Commit:

-1a2f0e9bf6905ace4096a5a67cfab0d383881eb3
+02638021ed683c87bd3170cb8f872060f3dfad91

Diff:

Revision 4 (+324)

Show changes

Checks run (2 failed)

flake8 failed.
JSHint failed.

flake8

JSHint

brennie
  1. 
      
  2. reviewboard/messagebus/base.py (Diff revision 4)
     
     
    Show all issues

    This file is missing a file-level docstring.

  3. reviewboard/messagebus/base.py (Diff revision 4)
     
     
    Show all issues

    Missing a class-level docstring.

    This class should inheirit from object.

  4. reviewboard/messagebus/base.py (Diff revision 4)
     
     
    Show all issues

    unicode.

  5. reviewboard/messagebus/base.py (Diff revision 4)
     
     
    Show all issues

    Remove this blank line.

  6. reviewboard/messagebus/base.py (Diff revision 4)
     
     
     
    Show all issues

    Blank line between these.

  7. reviewboard/messagebus/base.py (Diff revision 4)
     
     
    Show all issues

    We should use six.iteritems(obj) because in Python 2, dict.items returns a list instead of a generator.

    You can get six from:

    from django.utils import six
    
  8. reviewboard/messagebus/base.py (Diff revision 4)
     
     
     
    Show all issues

    Blank line between these.

  9. reviewboard/messagebus/base.py (Diff revision 4)
     
     
    Show all issues

    Missing a docstring.

    1. Dropped as function no longer exists in current version.

  10. reviewboard/settings.py (Diff revision 4)
     
     
    Show all issues

    This list should be kept in alphabetical order.

  11. Show all issues

    Blank line between ) and {. Same for all cases below.

  12. Show all issues

    Space after //. Missing period.

  13. Show all issues

    You can use an arrow function here, e.g.

    setTimeout(() => model._loadUpdates(), this._intervalMS);
    
  14. Show all issues

    This should be a member of the RB namespace e.g.

    RB.MessageBusRequest = function MessageBusRequest() {};
    

    Also, this might be a good place to use a Backbone model?

    1. Currently just using this for testing purposes, going to work in a proper Backbone model for MessageRequests soon.

  15. 
      
NI
Review request changed

Change Summary:

Created a message provider to handle review request updates.

Description:

   

On the client side message requests can be registered at a desired time

    interval. The minimum desired time interval is the interval in which
~   message requests are made to MessageBus, at url /messagebus/. Currently,
~   registering requests does not have an effect on the data being returned
~   from the server (a test request is made instead).

  ~ message requests are made to MessageBus, at url /messagebus/. There is
  ~ currently no handling of the payload response. A MessageRequest called
  ~ ReviewRequestUpdatesMessageRequest was created to handle storing
  + relevant data (namely the review request id), which is sent alongside the
  + associated message_provider_id as parameters of the request to the
  + message bus.

   
~  

On the server side, MessageProviders can be registered through MessageBus

~   to a registry. MessageBus requests will seperate individual message
~   requests, and pass the corresponding request to their appropriate
~   MessageProvider, if it exists (currently, there is no error handling if the
~   MessageProvider doesn't exist). The MessageProvider handles getting any
~   data, and returns it to MessageBus to be aggregrated and sent back to the
~   client.

  ~

On the server side, requests, identified by their message_provider_id,

  ~ pass their data to the appropriate MessageProvider, which is responsible
  ~ for accumulating messages and writing the data to the payload. Currently,
  ~ a message provider has been created for review request updates, ultimately
  ~ to replace the existing functionality of the ReviewRequestUpdatesView.
  ~ This MessageProvider returns the exact same data as you would expect by
  ~ going to the url /r/<#>/_updates.

Testing Done:

~  

Created a test MessageProvider and registered it to MessageBus. There is

~   some work done on registering message requests, but it isn't currently in
~   use. A test request is made to the MessageBus, and the request is passed to
~   the test MessageProvider. Different MessageProviders (and message requests)
~   can be made. The test request is currently only processed to return a
~   string, but in practice could be used to query the web api.

  ~

A ReviewRequestUpdatesMessageRequest was created and registered to the

  ~ client side message bus. The request is received by the server side
  ~ message bus, and the request arguments are passed to the
  ~ ReviewRequestUpdatesMessageProvider. No entries are provided in the
  ~ request arguments. The server responds with the review request update
  ~ data, which consists of two payloads which match up exactly with what
  + url /r/<#>/_updates would output (message bus is going to replace this,
  + so consistency between the two is important).

Commit:

-02638021ed683c87bd3170cb8f872060f3dfad91
+47a53a8ed141df763e02daf3ca6a04f0f55d548f

Diff:

Revision 5 (+630)

Show changes

Checks run (2 failed)

flake8 failed.
JSHint failed.

flake8

JSHint

NI
Review request changed

Change Summary:

Moved much of the entry update logic from the ReviewRequestPage model to the ReviewRequestUpdatesMessageRequest.

Commit:

-47a53a8ed141df763e02daf3ca6a04f0f55d548f
+ac1dc289c6e24e17e7502a410e414b0dbe053053

Diff:

Revision 6 (+817 -167)

Show changes

Checks run (2 failed)

flake8 failed.
JSHint failed.

flake8

JSHint

brennie
  1. 
      
  2. Show all issues

    MBRequest doesn't seem to be a thing (here and elsewhere). Do you mean RB.MessageRequest?

    1. Yep, use to have it named MBRequest. Updated it to be MessageRequest.

  3. 
      
NI
Review request changed

Change Summary:

Finished the MessageProvider and MessageRequest for review request updates.

Testing Done:

~  

A ReviewRequestUpdatesMessageRequest was created and registered to the

~   client side message bus. The request is received by the server side
~   message bus, and the request arguments are passed to the
~   ReviewRequestUpdatesMessageProvider. No entries are provided in the
~   request arguments. The server responds with the review request update
~   data, which consists of two payloads which match up exactly with what
~   url /r/<#>/_updates would output (message bus is going to replace this,
~   so consistency between the two is important).

  ~

A ReviewRequestUpdatesMessageRequest was created and registered to the

  ~ client side message bus. The request is received by the server side
  ~ message bus, and the request arguments are passed to the
  ~ ReviewRequestUpdatesMessageProvider. Any entries registered to the
  ~ message request have their payloads properly returned. Not providing an
  ~ entry will request all the entries. The data returned, which consists of
  ~ two payloads which match up exactly with what url /r/<#>/_updates would
  ~ output (message bus is going to replace this, so consistency between the
  + two is important).

Commit:

-ac1dc289c6e24e17e7502a410e414b0dbe053053
+0101a54b86b91a22d3f5dd640d9d1e671e866b73

Diff:

Revision 7 (+892 -337)

Show changes

Checks run (2 failed)

flake8 failed.
JSHint failed.

flake8

JSHint

NI
Review request changed

Change Summary:

Fixed ReviewBot complaints.

Commit:

-0101a54b86b91a22d3f5dd640d9d1e671e866b73
+b5d99a29a23561a952aed05019f90019672969b3

Diff:

Revision 8 (+892 -337)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

NI
david
Review request changed

Status: Discarded

Loading...