Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Feature/additional space and message type fields#153

Open
seba-aln wants to merge 15 commits intomasterpubnub/python:masterfrom
feature/additional-space-and-message-type-fieldspubnub/python:feature/additional-space-and-message-type-fieldsCopy head branch name to clipboard
Open

Feature/additional space and message type fields#153
seba-aln wants to merge 15 commits intomasterpubnub/python:masterfrom
feature/additional-space-and-message-type-fieldspubnub/python:feature/additional-space-and-message-type-fieldsCopy head branch name to clipboard

Conversation

@seba-aln
Copy link

No description provided.

tests/integrational/native_sync/test_publish.py Outdated Show resolved Hide resolved
@seba-aln seba-aln force-pushed the feature/additional-space-and-message-type-fields branch from ce4d004 to 1109fad Compare February 9, 2023 10:31
@seba-aln seba-aln force-pushed the feature/additional-space-and-message-type-fields branch from 64eb34a to 6c2e89a Compare February 27, 2023 08:44
Sebastian Molenda added 2 commits February 27, 2023 10:04
@seba-aln seba-aln force-pushed the feature/additional-space-and-message-type-fields branch from 6c2e89a to 00e1b16 Compare February 27, 2023 09:05
cleanup + add acceptance test action
@seba-aln seba-aln force-pushed the feature/additional-space-and-message-type-fields branch from 00e1b16 to 4b4776e Compare February 27, 2023 16:03
pubnub/endpoints/fetch_messages.py Show resolved Hide resolved
params = {'max': int(self._count)}
params = {
'max': int(self._count),
'include_type': 'true' if self._include_message_type else 'false',
Copy link
Contributor

Choose a reason for hiding this comment

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

'include_type': str(self._include_message_type).lower()

params = {
'max': int(self._count),
'include_type': 'true' if self._include_message_type else 'false',
'include_message_type': 'true' if self._include_message_type else 'false',
Copy link
Contributor

Choose a reason for hiding this comment

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

'include_type': str(self._include_message_type).lower()

if self._include_message_type is not None:
params['include_message_type'] = "true" if self._include_message_type else "false"
if self._include_space_id is not None:
params['include_space_id'] = "true" if self._include_space_id else "false"
Copy link
Contributor

Choose a reason for hiding this comment

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

params['include_space_id'] = str(self. _include_space_id).lower()

Copy link
Author

Choose a reason for hiding this comment

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

space_id can be null and in this case the parameter should not be added, that's why there's if statement

pubnub/endpoints/file_operations/publish_file_message.py Outdated Show resolved Hide resolved
@@ -0,0 +1,27 @@
class PNMessageType:
Copy link
Contributor

Choose a reason for hiding this comment

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

This class look like a good example of enum class:
https://docs.python.org/3/library/enum.html

Copy link
Author

Choose a reason for hiding this comment

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

Well Enum is a closed set of values - this is more like a wrapper around two independent message types:

  • One is user-defined and can be any string (with some constrains on server side)
  • Second is our own internal message type, which is numeric
    In both cases we want to deliver to client a string. If he didn't defined his own message type, we return "mapped" version of our internal message type.
    Either way we also allow access to "raw" values using PNMessageType.message_type and PNMessageType.pn_message_type properties. And i guess enum doesn't have such possibilities

Copy link

Choose a reason for hiding this comment

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

Every time we need to explain this I'm wondering if that's actually a good decision. Especially when you put it like that:

wrapper around two independent message types.

pubnub/models/consumer/history.py Outdated Show resolved Hide resolved
@seba-aln seba-aln force-pushed the feature/additional-space-and-message-type-fields branch from 52774ec to da677ba Compare March 1, 2023 09:23
@@ -0,0 +1,27 @@
class PNMessageType:
Copy link

Choose a reason for hiding this comment

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

Every time we need to explain this I'm wondering if that's actually a good decision. Especially when you put it like that:

wrapper around two independent message types.

pubnub/models/consumer/message_type.py Outdated Show resolved Hide resolved
tests/acceptance/history/environment.py Show resolved Hide resolved
.github/workflows/run-tests.yml Show resolved Hide resolved
@seba-aln seba-aln force-pushed the feature/additional-space-and-message-type-fields branch from d443f63 to c35ba87 Compare March 29, 2023 09:00
Copy link

@kleewho kleewho left a comment

Choose a reason for hiding this comment

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

I think it looks good

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.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.