New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require event format version to parse or create events #4470

Merged
merged 4 commits into from Jan 25, 2019

Conversation

Projects
None yet
3 participants
@erikjohnston
Copy link
Member

erikjohnston commented Jan 25, 2019

The vast majority of this PR is simply adding in the format version when parsing events from federation. Otherwise, the actual changes are just the ones in synapse/events/__init__.py

This was #4451 before I accidentally hit merge on that one....

@erikjohnston erikjohnston requested a review from matrix-org/synapse-core Jan 25, 2019

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jan 25, 2019

Codecov Report

Merging #4470 into develop will decrease coverage by <.01%.
The diff coverage is 76.25%.

@@             Coverage Diff             @@
##           develop    #4470      +/-   ##
===========================================
- Coverage    74.77%   74.76%   -0.01%     
===========================================
  Files          336      336              
  Lines        34031    34071      +40     
  Branches      5534     5538       +4     
===========================================
+ Hits         25446    25474      +28     
- Misses        7015     7025      +10     
- Partials      1570     1572       +2
@richvdh
Copy link
Member

richvdh left a comment

looks plausible I think

@@ -58,7 +85,29 @@ def create_event_id(self):

return e_id.to_string()

def new(self, key_values={}):
def new(self, room_version, key_values={}):

This comment has been minimized.

@richvdh

richvdh Jan 25, 2019

Member

one to fix another time, but this default for key_values looks disastrous, since we modify it and the modifications will get reused next time it is called.

This comment has been minimized.

@erikjohnston

erikjohnston Jan 25, 2019

Author Member

Good point! Will fix that in another PR

@erikjohnston erikjohnston merged commit b6dce9b into develop Jan 25, 2019

5 checks passed

ci/circleci: sytestpy2merged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy2postgresmerged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy3merged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy3postgresmerged Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

erikjohnston added a commit that referenced this pull request Jan 29, 2019

Fix receiving events from federation via a worker
This bug was introduced in PR #4470, commit 678a92c
@erikjohnston

This comment has been minimized.

Copy link
Member Author

erikjohnston commented Jan 29, 2019

This PR introduced a bug fixed in b6b73a0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment