Skip to content
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

Update README.md to add in 'publish message to channel including metadata' #545

Merged
merged 2 commits into from
Nov 29, 2023

Conversation

cameron-michie
Copy link
Member

Add in 'publish message to channel including metadata', i.e. in the extras headers json, which I believe has to be done through calling the Message constructor.

Add in 'publish message to channel including metadata', i.e. in the extras headers json, which I believe has to be done through calling the Message constructor
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.

Looks good, thanks for adding this! a couple of minor suggestions though:

README.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@sacOO7 sacOO7 left a comment

Choose a reason for hiding this comment

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

Can you add integration test for this?
You can add it below

async def test_subscribe(self):

@owenpearson
Copy link
Member

@sacOO7 we already have tests for publishing with extras

# TM2i, RSL6a2, RSL1h
async def test_publish_extras(self):
channel = self.ably.channels[
self.get_channel_name('canpublish:extras_channel')]
extras = {
'push': {
'notification': {"title": "Testing"},
}
}
message = Message(name='test-name', data='test-data', extras=extras)
await channel.publish(message)
# Get the history for this channel
history = await channel.history()
message = history.items[0]
assert message.name == 'test-name'
assert message.data == 'test-data'
assert message.extras == extras

@sacOO7
Copy link
Collaborator

sacOO7 commented Nov 29, 2023

Thanks for pointing out @owenpearson !

Added changed suggested to align with PEP8 and make it a bit clearer
@cameron-michie
Copy link
Member Author

Thanks very much guys, appreciate this. I have committed those changes Owen, and hope I've done it correctly!

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.

LGTM, thanks!

@owenpearson owenpearson merged commit 10cb4a0 into ably:main Nov 29, 2023
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants