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

fix: keep client default client read limit when realtime doesn't specify #631

Merged
merged 8 commits into from
Feb 2, 2024

Conversation

AndyTWF
Copy link
Contributor

@AndyTWF AndyTWF commented Feb 1, 2024

This is mostly for internal purposes, where we have MaxMessageSize set to 0 (read: unlimited). When this happens, the SDK should keep whatever limit it currently has set.

Previously it was trying to set 0 which prevented anything from being read from the connection/

This is mostly for internal purposes, where we have rate limits of 0 (read: unlimited).

When 0 is received from realtime, the SDK should continue with its existing defaults.
@AndyTWF AndyTWF requested a review from sacOO7 February 1, 2024 12:58
@github-actions github-actions bot temporarily deployed to staging/pull/631/features February 1, 2024 12:59 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/631/godoc February 1, 2024 12:59 Inactive
@AndyTWF AndyTWF requested a review from sacOO7 February 1, 2024 13:59
@github-actions github-actions bot temporarily deployed to staging/pull/631/features February 1, 2024 13:59 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/631/godoc February 1, 2024 13:59 Inactive
ably/realtime_channel_integration_test.go Outdated Show resolved Hide resolved
ably/realtime_channel_integration_test.go Outdated Show resolved Hide resolved
ably/realtime_channel_integration_test.go Outdated Show resolved Hide resolved
ably/realtime_channel_integration_test.go Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to staging/pull/631/features February 1, 2024 14:15 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/631/godoc February 1, 2024 14:15 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/631/features February 1, 2024 14:16 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/631/godoc February 1, 2024 14:17 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/631/features February 1, 2024 19:36 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/631/godoc February 1, 2024 19:36 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/631/features February 1, 2024 19:41 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/631/godoc February 1, 2024 19:42 Inactive
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.

LGTM

@sacOO7
Copy link
Collaborator

sacOO7 commented Feb 1, 2024

@AndyTWF do try running the test and check if limit is actually set ( with either logging or setting debug points )

@sacOO7 sacOO7 requested a review from lmars February 1, 2024 20:02
@AndyTWF
Copy link
Contributor Author

AndyTWF commented Feb 2, 2024

LGTM - nicely done 👍🏻

@AndyTWF AndyTWF merged commit 7a94a5a into main Feb 2, 2024
8 checks passed
@AndyTWF AndyTWF deleted the connection-read-limit-unlimited branch February 2, 2024 09:19
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.

2 participants