-
Notifications
You must be signed in to change notification settings - Fork 27
chore: rename realtime_channel.py and default_vcdiff_decoder.py #665
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
Conversation
Renamed files for consistency across SDK
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThis pull request refactors module organization for consistency. It renames Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
owenpearson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice 👍
08e12a1 to
154594a
Compare
also moved channeloptions to types
154594a to
c1f170e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/ably/realtime/realtimechannel_publish_test.py (1)
5-11: Fix import sorting to resolve linting failure.The pipeline is failing due to unsorted imports (I001). The
ably.*imports need to be alphabetically sorted.Proposed fix
-from ably.realtime.connection import ConnectionState from ably.realtime.channel import ChannelState +from ably.realtime.connection import ConnectionState -from ably.types.channeloptions import ChannelOptions from ably.transport.websockettransport import ProtocolMessageAction +from ably.types.channeloptions import ChannelOptions from ably.types.message import Message from ably.util.crypto import CipherParams from ably.util.exceptions import AblyException, IncompatibleClientIdException
🤖 Fix all issues with AI agents
In `@ably/realtime/realtime.py`:
- Around line 5-7: Reorder the ably imports in the realtime module to satisfy
I001 by sorting them alphabetically: import AblyRest from ably.rest.rest first,
then Channels from ably.realtime.channel, then Connection and ConnectionState
from ably.realtime.connection; update the import block in realtime.py to that
order so linters pass.
In `@ably/types/channeloptions.py`:
- Around line 62-66: The AblyException instantiation in ChannelOptions.from_dict
currently swaps the code and status_code arguments; update the call in from_dict
so the HTTP status code comes first and the Ably error code second (i.e.,
replace the current numeric order with 400 then 40000) so the exception becomes
AblyException("options must be a dictionary", 400, 40000).
- Around line 20-26: In ChannelOptions.__init__, the params validation
incorrectly uses a truthiness check which lets falsy non-dict values (like [])
pass and the AblyException is constructed with reversed arguments; change the
guard to "if self.__params is not None and not isinstance(self.__params, dict):"
and raise the exception with the correct argument order (swap the existing
arguments so the numeric code/status are passed in the expected positions) when
constructing AblyException for the params validation.
In `@test/ably/realtime/realtimechannel_test.py`:
- Around line 5-7: Reorder the imports to satisfy I001 by grouping and
alphabetizing them: place the ably.realtime.channel import (ChannelState,
RealtimeChannel) before ably.realtime.connection (ConnectionState), followed by
ably.types.channeloptions (ChannelOptions); adjust the import order so the three
from-import statements are in that alphabetical/grouped order.
In `@test/ably/realtime/realtimeresume_test.py`:
- Around line 5-7: Reorder the import statements so they satisfy I001 import
ordering: group and sort imports consistently (third-party first, then local)
and alphabetize within groups; specifically adjust the lines importing
ConnectionState, ChannelState, and ProtocolMessageAction so their modules are in
the correct order (e.g., import ProtocolMessageAction, then ConnectionState,
then ChannelState or alphabetically by module path) to eliminate the lint
error—update the three import lines around the references to ConnectionState,
ChannelState, and ProtocolMessageAction accordingly.
🧹 Nitpick comments (1)
ably/types/channeloptions.py (1)
32-35: Make params return type Optional for accuracy.
self.__paramscan beNone, but the annotation saysdict[str, str]. Consider reflecting the nullable return type.♻️ Suggested tweak
- def params(self) -> dict[str, str]: + def params(self) -> dict[str, str] | None:
owenpearson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good 👍
Renamed files for consistency across SDK
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.