Skip to content

RSN4b Compliance#474

Merged
AndyTWF merged 3 commits intointegration/realtimefrom
472-rest-rsn4b
Mar 3, 2023
Merged

RSN4b Compliance#474
AndyTWF merged 3 commits intointegration/realtimefrom
472-rest-rsn4b

Conversation

@AndyTWF
Copy link
Contributor

@AndyTWF AndyTWF commented Mar 3, 2023 •
edited
Loading

This change does the following:

  • Removes the __delitem__ magic method from the REST Channels implementation (as not part of spec)
  • Fixes a non-compliance with RSN4b - whereby previously calling release on a non-existent channel would result in a KeyError being raised.

Fixes #472

AndyTWF added 2 commits March 3, 2023 12:11
@AndyTWF
Remove del implementation
86df591 
Removing the magic method as it is not part of the specification.
@AndyTWF
fix: RSN4b compliance on rest channels
a820b46 
The rest channels were not compliant with the aformentioned spec point
as releasing a non-existent channel would raise an error.

This change implements RSN4b for the rest client.
@AndyTWF AndyTWF added the breaking Backwards incompatible changes made to the public API. label Mar 3, 2023
@github-actions github-actions bot temporarily deployed to staging/pull/474/features March 3, 2023 12:16 Inactive
@AndyTWF AndyTWF requested review from mohyour and owenpearson March 3, 2023 12:42
@AndyTWF AndyTWF marked this pull request as ready for review March 3, 2023 12:42
owenpearson
owenpearson approved these changes Mar 3, 2023
View reviewed changes
Copy link
Member

@owenpearson owenpearson left a comment

Choose a reason for hiding this comment

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

Nice one 🙂

@AndyTWF AndyTWF linked an issue Mar 3, 2023 that may be closed by this pull request
RSN4b: Error raised #472
Closed
@mohyour
Copy link
Contributor

mohyour commented Mar 3, 2023

I think this PR should be against the main branch. Is it against the integration/realtime because it's a breaking change?

@AndyTWF
Copy link
Contributor Author

AndyTWF commented Mar 3, 2023

I think this PR should be against the main branch. Is it against the integration/realtime because it's a breaking change?

Yes - as the change is breaking we thought it best to do it against the realtime branch so it goes out as part of the 2.0 release :)

@AndyTWF AndyTWF merged commit 062ac34 into integration/realtime Mar 3, 2023
@AndyTWF AndyTWF deleted the 472-rest-rsn4b branch March 3, 2023 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@owenpearson owenpearson owenpearson approved these changes

+1 more reviewer

@mohyour mohyour mohyour approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

breaking Backwards incompatible changes made to the public API.

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

RSN4b: Error raised

3 participants

@AndyTWF @mohyour @owenpearson