Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Jan 31, 2022

Allowing max_message_size for Realtime client. Client gets max_message_size from current connection.details. ConnectionDetails attributes are populated when connection is in CONNECTED state.

Added max_message_size to Ably::Rest::Client.

@ghost ghost self-assigned this Jan 31, 2022
@ghost ghost requested a review from owenpearson January 31, 2022 23:18
@ghost ghost marked this pull request as ready for review January 31, 2022 23:18
Copy link
Member

@owenpearson owenpearson left a comment

Choose a reason for hiding this comment

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

LGTM, thanks, although I would prefer to have a spec which mocks the server to respond with a different maxMessageSize and assert that the library sets its own max_message_size accordingly. Do you reckon this would be easy enough to do @lukaszsliwa ?

@ghost
Copy link
Author

ghost commented Feb 1, 2022

LGTM, thanks, although I would prefer to have a spec which mocks the server to respond with a different maxMessageSize and assert that the library sets its own max_message_size accordingly. Do you reckon this would be easy enough to do @lukaszsliwa ?

Thanks @owenpearson . I have prepared two more tests. Both for a case, when the server sends Action.CONNECTED and no maxMessageSize is provided.

Here is the example snippet, if the client wants (because of some reason) overwrite the server parameters:

client.connection.once(:connected) do
  connection_details = Ably::Models::ConnectionDetails.new(
    'maxMessageSize' => 1048576 # 1MB
    #...
  )
  client.connection.set_connection_details(connection_details)
  channel.publish('event', 'x' * 1048575)
end

Screenshot 2022-02-01 at 18 46 42

@ghost ghost marked this pull request as draft February 7, 2022 15:16
@owenpearson owenpearson marked this pull request as ready for review February 15, 2022 16:45
@owenpearson owenpearson merged commit c44c276 into main Feb 15, 2022
@owenpearson owenpearson deleted the TO3l8-max_message_size branch February 15, 2022 16:45
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.

1 participant