-
Notifications
You must be signed in to change notification settings - Fork 116
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
Add type hints to customer facing interfaces #195
Conversation
b84249e
to
a4f849f
Compare
|
||
|
||
class GrantToken(Endpoint): | ||
GRANT_TOKEN_PATH = "/v3/pam/%s/grant" | ||
|
||
def __init__(self, pubnub): | ||
def __init__(self, pubnub, channels: Union[str, List[str]] = None, channel_groups: Union[str, List[str]] = None, | ||
users: Union[str, List[str]] = None, spaces: Union[str, List[str]] = None, |
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.
Why do you need users
and spaces
parameters?
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.
the users and spaces were already in there so I just bubbled them up to the constructor
|
||
self._sort_params = True | ||
|
||
def ttl(self, ttl): | ||
def ttl(self, ttl: int) -> 'GrantToken': |
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.
Is it a common pattern to extract a variable that repeats into constants?
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.
yeah. this is python's way of saying :
def ttl(self, ttl): # <-this is a setter
self._ttl # <- this is a "protected" property
|
||
|
||
class GetMessageActions(Endpoint): | ||
GET_MESSAGE_ACTIONS_PATH = '/v1/message-actions/%s/channel/%s' | ||
MAX_LIMIT = 100 | ||
|
||
def __init__(self, pubnub): | ||
def __init__(self, pubnub, channel: str = None, start: str = None, end: str = None, limit: str = None): |
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.
I assume it's not possible to change it, so let's leave it. But I noticed that perhaps all responses contain a reference to a PubNub object. Is there any benefit from such hard coupling?
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.
it's better than having a global variable because you may have more than one instance running at the same time
a4f849f
to
2a11d0e
Compare
2a11d0e
to
ab32531
Compare
@pubnub-release-bot release |
🚀 Release successfully completed 🚀 |
feat: Reconnecting with exponential backoff by default.
Automatic reconnecting for subscribe with exponential backoff is now enabled by default.
BREAKING CHANGES: Modifies Default Behavior of SDK
feat: Access manager v2 deprecation
Access manager v2 endpoints (grant and audit) will no longer be supported after December 31, 2024, and will be removed without further notice. Refer to the documentation to learn more.
feat: Configuration Becomes Immutable
Once used to instantiate PubNub, the configuration object (PNConfiguration instance) becomes immutable. You will receive exceptions if you rely on modifying the configuration after the PubNub instance is created. Refer to the documentation to learn more.
BREAKING CHANGES: Modifies Default Behavior of SDK
style: Type hints and named parameters
Type hints for parameters and return values are now added to provide a better developer experience.
style: Named parameters for endpoints
All endpoints are now accessible through the builder pattern and named parameters, providing a more flexible experience suitable for custom solutions.