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

Add room_version param to get_pdu #4448

Merged
merged 3 commits into from Jan 24, 2019

Conversation

Projects
None yet
3 participants
@erikjohnston
Copy link
Member

erikjohnston commented Jan 23, 2019

When we add new event format we'll need to know the event format or room
version when parsing events.

erikjohnston added some commits Jan 23, 2019

Add room_version param to get_pdu
When we add new event format we'll need to know the event format or room
version when parsing events.
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jan 23, 2019

Codecov Report

Merging #4448 into develop will increase coverage by 1.16%.
The diff coverage is 70.37%.

@@            Coverage Diff             @@
##           develop   #4448      +/-   ##
==========================================
+ Coverage    73.64%   74.8%   +1.16%     
==========================================
  Files          302     336      +34     
  Lines        29818   34804    +4986     
  Branches      4895    5800     +905     
==========================================
+ Hits         21960   26036    +4076     
- Misses        6426    7149     +723     
- Partials      1432    1619     +187
@erikjohnston

This comment has been minimized.

Copy link
Member Author

erikjohnston commented Jan 24, 2019

Split out from #4403 and address this review comment about making sure get_pdu knows the correct room version

@richvdh
Copy link
Member

richvdh left a comment

looks generally sane, modulo question below

@@ -1659,6 +1664,8 @@ def _persist_auth_tree(self, origin, auth_events, state, event):
create_event = e
break

room_version = create_event.content.get("room_version", RoomVersions.V1)

This comment has been minimized.

@richvdh

richvdh Jan 24, 2019

Member

is it possible for us to get here without a create_event (if the auth chain is faulty), in which case this will explode rather than failing sensibly?

This comment has been minimized.

@erikjohnston

erikjohnston Jan 24, 2019

Author Member

I don't believe so, but may as well add a check anyway.

break

if room_version is None:
# We use this error has that is what

This comment has been minimized.

@richvdh

richvdh Jan 24, 2019

Member

deep.

@erikjohnston erikjohnston merged commit edc1e21 into develop Jan 24, 2019

4 of 5 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment