Repository navigation
Fix SDK bugs found by the live docs snippet tests - #242
Open
techwritermat wants to merge 3 commits into
Open
techwritermat wants to merge 3 commits into
techwritermat wants to merge 3 commits into
Conversation
* PubNubSubscriptionSet.unsubscribe() now unsubscribes the channels of the set
and removes the set from the registry. It sent an empty unsubscribe before.
* unsubscribe_all() resets the registry to dicts. It set a list, so the next
subscribe() raised TypeError and a second unsubscribe_all() AttributeError.
* subscriptions and sets get channel_metadata, user_metadata, membership and
file_message methods. The registry callback called methods that did not
exist, so the sync message worker raised on the first App Context or file
event.
* subscription.add_listener() calls SubscribeCallback listeners with
(pubnub, event) like the client does. It raised TypeError.
* PubNubUserMetadata sets _type, it set the unused _types.
* message action events expose event ("added" or "removed").
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ging * get_message_actions sends end when start is set, limit accepts int or str. * pubnub.reconnect() calls the subscription manager and no longer raises AttributeError. Added pubnub.disconnect(). * asyncio get_subscribed_channels(), get_subscribed_channel_groups() and unsubscribe_all() work before the first subscribe (PNContext starts empty). * if_matches_etag() no longer leaks the If-Match header into later requests (the headers dict was shared by the class). * total_count=True in MembershipIncludes and MemberIncludes requests a count. The two include_object cassettes now record count=True. * parse_token() raises PubNubException for every malformed token. * grant() counts delete as a flag. * fetch_messages items parse custom_message_type. * setup.py: aiohttp>=3.10.11, the old range did not overlap the dev pin. Adds offline regression tests in tests/unit. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
The example passed start below end. The SDK dropped end, so the call worked by accident. Now that get_message_actions sends both bounds the service answers HTTP 400 "Start must be greater than or equal to end". start is the newer bound (exclusive) and end the older bound (inclusive). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
parfeon
approved these changes
Nov 12, 2026
parfeon
left a comment
parfeon
left a comment
Contributor
There was a problem hiding this comment.
Mostly looks good but still had some small questions.
| def limit(self, limit: str) -> 'GetMessageActions': | ||
| assert isinstance(limit, str) | ||
| self._limit = limit | ||
| assert isinstance(limit, (str, int)) and not isinstance(limit, bool) |
Contributor
There was a problem hiding this comment.
Isn't our current contract according to the docs is str?
| itself, so there this call does nothing. | ||
| """ | ||
| self._validate_subscribe_manager_enabled() | ||
| return self._subscription_manager.reconnect() |
Contributor
There was a problem hiding this comment.
This will work only for NativeSubscriptionManager and not implemented for EventEngineSubscriptionManager (pubnub_asyncio)
Comment on lines
+424
to
+427
| if not hasattr(self._subscription_manager, 'disconnect'): | ||
| raise PubNubException( | ||
| errormsg="disconnect() is not supported by %s" % type(self._subscription_manager).__name__) | ||
| return self._subscription_manager.disconnect() |
Contributor
There was a problem hiding this comment.
Situation similar to the reconnect. Should we add same exception for reconnect?
Comment on lines
+43
to
+47
| @staticmethod | ||
| def _set_origin(result, channel, subscription): | ||
| # subscription and subscription set listeners match App Context events by these attributes | ||
| result.channel = channel | ||
| result.subscription = subscription |
Contributor
There was a problem hiding this comment.
I'm curious, why we can't follow the same pattern as we have with PNSignalMessageResult, PNMessageActionResult, and PNFileMessageResult for PNChannelMetadataResult, PNUUIDMetadataResult, and PNMembershipResult constructors where channel and subscription_match passed as one of the constructor arguments?
| "request": { | ||
| "method": "PATCH", | ||
| "uri": "https://ps.pndsn.com/v2/objects/{PN_KEY_SUBSCRIBE}/uuids/someuuid/channels?include=custom%2Cstatus%2Ctype%2CtotalCount%2Cchannel%2Cchannel.custom%2Cchannel.type%2Cchannel.status", | ||
| "uri": "https://ps.pndsn.com/v2/objects/{PN_KEY_SUBSCRIBE}/uuids/someuuid/channels?include=custom%2Cstatus%2Ctype%2CtotalCount%2Cchannel%2Cchannel.custom%2Cchannel.type%2Cchannel.status&count=True", |
Contributor
There was a problem hiding this comment.
Why count=True had to be added? Also why True and not true?
REST API docs say that by default it should be omitted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes 15 bugs found while writing live tests for the Python and asyncio docs samples (PR #241). Each bug was reproduced against the real service or by calling the SDK, except the ones marked "code only". Two commits: subscriptions and listeners, then the rest. No public signature changes.
Fixes
get_message_actionsdropsendwhenstartis set,limit()accepts onlystrpubnub/endpoints/message_actions/get_message_actions.pyPubNubSubscriptionSet.unsubscribe()sends an empty unsubscribe, channels stay subscribedpubnub/models/subscription.pyuuid,channel,membership,file_message), the sync worker raises on the first App Context or file eventpubnub/models/subscription.py,pubnub/workers.pyunsubscribe_all()sets a list where a dict is used, nextsubscribe()raises TypeErrorpubnub/models/subscription.pysubscription.add_listener(SubscribeCallback)raises TypeErrorpubnub/models/subscription.pyPubNubUserMetadatasets_typesinstead of_typepubnub/models/subscription.pypubnub.reconnect()always raises AttributeError, nodisconnect()pubnub/pubnub_core.pyget_subscribed_channels()andunsubscribe_all()raise AttributeError before the first subscribepubnub/event_engine/models/states.pyif_matches_etag()leaks theIf-Matchheader into every later request (class-level dict), a plain set returned 412pubnub/endpoints/objects_v2/objects_endpoint.pyMembershipIncludes(total_count=True)andMemberIncludes(total_count=True)do not request a countpubnub/endpoints/objects_v2/objects_endpoint.pyaddedorremovedpubnub/models/consumer/pubsub.py,pubnub/workers.pyparse_token()raisesbinascii.ErrororTypeErroron malformed tokenspubnub/managers.pygrant()ignoresdeletein the "at least one flag" checkpubnub/endpoints/access/grant.pyfetch_messagesitems never parsecustom_message_typepubnub/models/consumer/history.pysetup.pyaiohttp>3.10.11does not overlap the dev pin>=3.10.11,<3.11, pip cannot resolve bothsetup.pyTests
tests/unit/(test_subscription_registry.py,test_message_worker_events.py,test_get_message_actions_params.py,test_sdk_fixes.py)channel_memberships_with_include_object.jsonandchannel_members_with_include_object.jsonnow recordcount=True, because the total count is now requestedPN_KEY_*values so the recorded tests replay: unit, functional and integration give 1032 passed and 46 failed before, 1089 passed and 46 failed after. The 46 failures are identical before and after and need the real service (file upload, asyncio and threaded subscribe, message actions integration, two event engine tests)get_message_actionsand the set unsubscribe (sync and asyncio) failedExample fixed because of the
get_message_actionschangeexamples/native_sync/message_reactions.py(run bytests/examples/native_sync/test_examples.py::test_message_reactions) passedstartbelowend. The SDK droppedend, so it worked by accident and was flaky. With both bounds sent the service answers HTTP 400 "Start must be greater than or equal to end". The example now uses the newest reaction plus one asstart(newer, exclusive) and the message timetoken asend(older, inclusive). Passes 5 of 5 against the real service with the demo keys.Needs a maintainer decision (not changed here)
filter_expression(pubnub/event_engine/effects.py)enable_presence_heartbeaton (pubnub/pubnub_asyncio.py:624-630), sync always doesshould_storedefaults to 0 for files (pubnub/endpoints/file_operations/publish_file_message.py:96), the docs say truecrypto_moduleis set (send_file.py:44-53), this one is a security questionstop()SDK_VERSIONis 10.4.0,.pubnub.ymlsays 10.8.1 (release tooling)custom=keyword does not implyinclude_custom, the builder.custom()doesreconnect()anddisconnect()are no-ops or raise on the asyncio default event engine managersubscription.add_listener()receive events of every subscription, sets never receive presenceNo version bump or changelog entry, left to release.
🤖 Generated with Claude Code