Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Move command processing out of transport #7134

Closed
wants to merge 2 commits into from

Conversation

erikjohnston
Copy link
Member

@erikjohnston erikjohnston commented Mar 24, 2020

This moves out the command processing from protocol and merges it with the logic in client and resource. This simplifies things quite a bit and merges the server and client handling into one, rather than duplicating a bunch of code.

I'd suggest ignoring much of the changes in protocol.py, client.py and resource.py, most of it is just moving the code into handler.py.

This PR does a few things on top of the move:

  1. Removes a bunch of unused functions around handling SYNC command (we probably just want to remove the command at this point).
  2. Removes the checks for whether a command is a valid client or server command. These checks become a bit pointless in a redis world.
  3. Changes how worker gets told about replication data. Instead of it sub classing the replication handler and overriding methods, we add a new "replication data handler" object to the HomeServer which worker apps can replace instead.

Based on #7024 and #7128

@erikjohnston erikjohnston force-pushed the erikj/repl_merge_client_server branch from 2dd2754 to 11f5579 Compare March 30, 2020 15:40
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, can you break this up? I can't follow it.


class GenericWorkerReplicationHandler(WorkerReplicationDataHandler):
def __init__(self, hs):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs a call to super().__init__


class GenericWorkerReplicationHandler(WorkerReplicationDataHandler):
def __init__(self, hs):
self.store = hs.get_datastore()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this overwrites a field in the base class

self.send_command(RdataCommand(stream_name, token, data))


class DummyReplicationDataHandler:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it'd be nice to define an AbstractReplicationDataHandler that defines the interface returned by get_replication_data_handler

@erikjohnston
Copy link
Member Author

Closing in favour of #7185

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants