search

ably / ably-php

PHP client library SDK for Ably realtime messaging service
https://ably.com/download
Apache License 2.0
50 stars 10 forks source link

Idempotent #58

Closed jdavid closed 5 years ago

jdavid commented 6 years ago

Not ready for merge as only 1 item is done (TO3n), but prefer to get early feedback.

jdavid commented 5 years ago

Has idempotent support been dropped from the dev environment? Because the RSL1k5 test is failing now.

jdavid commented 5 years ago

RSL1k4 done, the PR is complete.

But tests are failing. At least some of the failures are because, apparently, the dev environment doesn't support idempotency anymore.

jdavid commented 5 years ago

Tests are green now.

@mattheworiordan I think I've addressed your first 2 comments, but I need clarification on the third one. Otherwise the PR is complete.

jdavid commented 5 years ago

ping

mattheworiordan commented 5 years ago

@jdavid when you say the third one, I assume you are referring to https://github.com/ably/ably-php/pull/58#discussion_r221802300. If so, I am afraid I am waiting on @paddybyers. I will ping him again too.

mattheworiordan commented 5 years ago

Better yet. Can you not just provide an http raw log?

On Wed, 17 Oct 2018 at 12:53, Paddy Byers notifications@github.com wrote:

@paddybyers commented on this pull request.

In tests/ChannelIdempotentTest.php https://github.com/ably/ably-php/pull/58#discussion_r225892179:

  • $msg = new Message();
  • $msg->name = 'name';
  • $msg->data = 'data';
  • $msg->id = 'foobar';
  • $messages[] = $msg;
  • $msg = new Message();
  • $msg->name = 'name';
  • $msg->data = 'data';
  • $messages[] = $msg;
  • $body = $channel->__publish_request_body( $messages );
  • $body = json_decode($body);
  • $this->assertEquals( $body[0]->id, "foobar" );
  • $this->assertFalse( property_exists($body[1], 'id') );

@jdavid https://github.com/jdavid ok then, how easy is it for me to get set up to run the tests myself to see what's going on?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ably/ably-php/pull/58#discussion_r225892179, or mute the thread https://github.com/notifications/unsubscribe-auth/AACrDeB72K3Udnpna_hNGu7Fcz3iZ8cVks5ulxougaJpZM4Wg8G5 .

-- Matthew O'Riordan https://linkedin.com/in/lemon Ably CEO & tech co-founder https://www.ably.io/about/team OpenAPI Board Member https://www.openapis.org/membership/members github https://github.com/mattheworiordan / twitter https://twitter.com/mattheworiordan / linkedin https://linkedin.com/in/lemon [image: Logo] https://www.ably.io/

Recent blog: https://blog.ably.io/boosting-developer-experience-on-our-website-with-a-major-facelift-a122e7bb4ce7Boosting Developer Experience on our website with a major facelift https://blog.ably.io/boosting-developer-experience-on-our-website-with-a-major-facelift-a122e7bb4ce7

jdavid commented 5 years ago

Sorry the test was not actually publishing to the server, now it does and it is rejected.

Now is it good?

paddybyers commented 5 years ago

Sorry the test was not actually publishing to the server, now it does and it is rejected. Now is it good?

Looks good to me, thanks