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

Added subscription unsubscribing info #3654

Merged

Conversation

Bingdom
Copy link
Contributor

@Bingdom Bingdom commented Oct 1, 2024

Description

I added some documentation to detect unsubscribing from subscriptions, to the documentation.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Documentation

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

Summary by Sourcery

Update the documentation to include information on unsubscribing from subscriptions in GraphQL, demonstrating the process using Strawberry and Apollo-client.

Documentation:

  • Add documentation on how to unsubscribe from subscriptions in GraphQL using Strawberry and Apollo-client.

Copy link
Contributor

sourcery-ai bot commented Oct 1, 2024

Reviewer's Guide by Sourcery

This pull request adds documentation about unsubscribing from subscriptions in GraphQL, specifically for Strawberry. The changes include examples of how to unsubscribe in Apollo client and how to handle unsubscription events in Strawberry using Python.

No sequence diagrams generated as the changes look simple and do not need a visual representation.

File-Level Changes

Change Details Files
Added documentation for unsubscribing from subscriptions
  • Introduced a new section titled 'Unsubscribing subscriptions'
  • Provided an example of unsubscribing in Apollo client using JavaScript
  • Demonstrated how to handle unsubscription in Strawberry using Python
  • Explained the use of asyncio.CancelledError for detecting unsubscriptions
  • Showed how to manage active subscribers using a dictionary
docs/general/subscriptions.md
Minor code formatting change
  • Removed a trailing comma in the GraphQLWsLink configuration
docs/general/subscriptions.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @Bingdom - I've reviewed your changes - here's some feedback:

Overall Comments:

  • There's a typo in the word 'supoprts' which should be corrected to 'supports'.
  • Consider restructuring the documentation to clearly separate client-side (Apollo) and server-side (Strawberry) examples for better clarity.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟡 Documentation: 3 issues found

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@@ -254,6 +254,52 @@ schema = strawberry.Schema(query=Query, subscription=Subscription)

[pep-525]: https://www.python.org/dev/peps/pep-0525/

## Unsubscribing subscriptions

In GraphQL, it is possible to unsubscribe from a subscription. Strawberry supoprts this behaviour, and is done using a `try...except` block.
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (documentation): Fix typo in 'supports'


In GraphQL, it is possible to unsubscribe from a subscription. Strawberry supoprts this behaviour, and is done using a `try...except` block.

In Apollo-client, closing a subscription can be acheived like the following:
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (documentation): Fix typo in 'achieved'

Comment on lines 281 to 290
event_messages = {}

@strawberry.type
class Subscription:
@strawberry.subscription
async def message(self) -> AsyncGenerator[int, None]:
try:
subId = uuid4()

event_messages[subId] = []
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (documentation): Clarify the usage of UUID as dictionary key

The code uses a UUID as a key for the event_messages dictionary, but it's not clear where this UUID comes from or how it's generated. Consider adding a brief explanation or comment about this.

Copy link
Member

@patrick91 patrick91 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 to me, but let's double check with @DoctorJohn

Copy link
Member

@DoctorJohn DoctorJohn left a comment

Choose a reason for hiding this comment

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

Thanks for adding this to the docs @Bingdom, had to post examples like this many times on Discord in the past few years. I made only one small suggestion, everything else is good to go :)

docs/general/subscriptions.md Outdated Show resolved Hide resolved
@DoctorJohn DoctorJohn merged commit 943736f into strawberry-graphql:main Oct 1, 2024
4 checks passed
@botberry
Copy link
Member

botberry commented Oct 1, 2024

Thanks for contributing to Strawberry! 🎉 You've been invited to join
the Strawberry GraphQL organisation 😊

You can also request a free sticker by filling this form: https://forms.gle/dmnfQUPoY5gZbVT67

And don't forget to join our discord server: https://strawberry.rocks/discord 🔥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants