ably-common integration #6
Conversation
Updated tests to use fixture shared across platforms
Removed superfluous CapabilityTest suite (unrelated to client library)
|
It looks pretty good. A few details on variable names but I think that's it. Since you have a commit mentioning an incompatibility for PHP < 5.6, maybe we should add it in our travis CI? |
|
Alright, just a sec. |
|
Ok so let's just say 5.3 for my peace of mind ;). |
Travis PHP version set to oldest supported by project - i.e. 5.3
|
@kouno So I started adding the getter for better readability and then I realized how messy the tests are (especially PHP 5.3 support is troublesome. It doesn't always have openssl bundled (required for AES) and has outdated versions of curl and whatnot, so it's behaving oddly. I'll have to investigate that later. |
|
Sigh, the tests didn't pass, because sandbox has crashed again. But, if you check the log, everything else is OK. |
Added more universal and mockable HTTP class Added logging class Added proper authUrl, authCallback support to Auth Fixed a lot of naming issues of parameters and previously ignored parameters (log level, log callback, authParams, ...) Preliminary support for fallback servers Added classes for options (AuthOptions, ClientOptions) and tokens (TokenParams, TokenRequest, TokenDetails) Updated AuthTest
Test for authUrl
… instead of issued_at, clientId instead of client_id, etc.) Bug fixes - auth token not base64 encoded, custom logger callback
|
Pushed the latest changes. I didn't realize this PR was still pending, so it got merged into this huge one. |
|
Added automatic token renewal. |
|
Looking good, nice to see some progress on both auth & fallbacks. |
|
The current logic is to try normal host and then fallback hosts (a-e). On failure, the first host that worked will be used from that point on throughout the session until there's another failure. |
|
This PR is a bit too big now. Shall we just merge it as it passes CI and address the feedback in a follow up PR? @mattheworiordan @bladeSk |
|
Absolutely. I could then submit the library to packagist and see if it's working properly with composer. |
|
In regards to the host shuffling, reading the code it is not obvious what is going on. I would definitely prefer that every request always tries the default host first, and then tries a random fallback host in order after. If you want to retain the last known good fallback host at the top of that list that is fine, however the initial request should always go to |
Tests now use shared fixtures from ably-common submodule
Pagination updated to match the new spec ably/ably-js#28 (comment)