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

Split event creation into a separate handler #2847

Merged
merged 5 commits into from
Feb 6, 2018

Conversation

erikjohnston
Copy link
Member

No description provided.

@erikjohnston
Copy link
Member Author

@matrixbot retest this please

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.

probably ought to update the copyright on some of these files

}
for user_id, profile in users_with_profile.iteritems()
})

@measure_func("_create_new_client_event")
@defer.inlineCallbacks
def _create_new_client_event(self, builder, requester=None, prev_event_ids=None):
Copy link
Member

Choose a reason for hiding this comment

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

if we're moving this anyway, and it's intended to be called from outside the handler, please can we drop the leading _ ?

self.ratelimiter = hs.get_ratelimiter()
self.notifier = hs.get_notifier()

# This is only used to get at ratelimit function, and maybe_kick_guest_users
Copy link
Member

Choose a reason for hiding this comment

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

I kinda wish it wish they were factored out, but I guess that is a problem for another time


class EventCreationHandler(object):
def __init__(self, hs):
self.hs = hs
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 if all these private fields were named with a leading underscore, but I suppose that increases the diff significantly?

Copy link
Member Author

Choose a reason for hiding this comment

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

Its doable, but the diff would be quite big

@@ -378,7 +378,7 @@ def create_event(self, requester, event_dict, token_id=None, txn_id=None,
if txn_id is not None:
builder.internal_metadata.txn_id = txn_id

event, context = yield self._create_new_client_event(
event, context = yield self.create_new_client_event(
Copy link
Member

Choose a reason for hiding this comment

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

yeah, you see you made the mistake of expecting the thing that is named like a private field to not be gut-wrenched across the codebase.

Copy link
Member

Choose a reason for hiding this comment

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

oh, no you didn't. you just did the other changes in a commit called "Update copyright" which ... I didn't expect.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, blast, I think that might have been vscode doing the changes without saving the open files. Sigh.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have unfucked it now.

@erikjohnston erikjohnston force-pushed the erikj/separate_event_creation branch from b165e0f to 3e1e69c Compare February 6, 2018 16:40
@richvdh richvdh assigned erikjohnston and richvdh and unassigned richvdh and erikjohnston Feb 6, 2018
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.

looks plausible!

@richvdh richvdh assigned erikjohnston and unassigned richvdh Feb 6, 2018
@erikjohnston erikjohnston merged commit 617199d into develop Feb 6, 2018
@erikjohnston erikjohnston deleted the erikj/separate_event_creation branch February 6, 2018 17:01
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