Skip to content

Conversation

Aschen
Copy link
Contributor

@Aschen Aschen commented Aug 5, 2019

What does this PR do?

Adds a connection timeout for http protocol

https://deploy-preview-439--doc-sdk-javascript.netlify.com/protocols/http/properties/

How should this be manually tested?

  • Step 1 : instantiate the protocol with a timeout new Http('localhost', { timeout: 1000 })
  • Step 2 :
  • Step 3 :

@codecov
Copy link

codecov bot commented Aug 6, 2019

Codecov Report

Merging #439 into 6-dev will decrease coverage by 0.05%.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff            @@
##            6-dev    #439      +/-   ##
=========================================
- Coverage   96.15%   96.1%   -0.06%     
=========================================
  Files          33      33              
  Lines        1612    1616       +4     
=========================================
+ Hits         1550    1553       +3     
- Misses         62      63       +1
Impacted Files Coverage Δ
src/protocols/http.js 90.34% <75%> (-0.44%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c68977...3a0a2cd. Read the comment docs.

@codecov
Copy link

codecov bot commented Aug 6, 2019

Codecov Report

Merging #439 into 6-dev will decrease coverage by 0.05%.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff            @@
##            6-dev    #439      +/-   ##
=========================================
- Coverage   96.15%   96.1%   -0.06%     
=========================================
  Files          33      33              
  Lines        1612    1616       +4     
=========================================
+ Hits         1550    1553       +3     
- Misses         62      63       +1
Impacted Files Coverage Δ
src/protocols/http.js 90.34% <75%> (-0.44%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c68977...41f3946. Read the comment docs.

| `port` | <pre>number</pre><br/>(`7512`) | Kuzzle server port |
| `sslConnection` | <pre>boolean</pre><br/>(`false`) | Use SSL to connect to Kuzzle server |
| `customRoutes` | <pre>object</pre><br/>(`{}`) | Add custom routes <SinceBadge version="6.2.0"/> |
| `timeout` | <pre>number</pre><br/>(`0`) | Connection timeout <SinceBadge version="6.2.1"/> |
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the unit? And does '0' mean "no timeout"?

| `port` | <pre>number</pre> | Kuzzle server port | Get |
| `protocol` | <pre>string</pre> | `https` or `http` | Get |
| `ssl` | <pre>boolean</pre> | `true` if ssl is active | Get |
| `timeout` | <pre>number</pre> | Connection timeout <SinceBadge version="6.2.1"/>| Get/Set |
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto.

Copy link
Member

@alexandrebouthinon alexandrebouthinon left a comment

Choose a reason for hiding this comment

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

According @scottinet review LGTM

Co-Authored-By: Sébastien Cottinet <[email protected]>
@scottinet scottinet merged commit f1e218d into 6-dev Aug 9, 2019
@scottinet scottinet deleted the add-timeout-support-to-http branch August 9, 2019 14:06
@scottinet scottinet mentioned this pull request Sep 11, 2019
scottinet added a commit that referenced this pull request Sep 11, 2019
# [6.2.2](https://github.com/kuzzleio/sdk-javascript/releases/tag/6.2.2) (2019-09-11)


#### Bug fixes

- [ [#437](#437) ] Fix duplicate subscriptions on reconnect   ([benoitvidis](https://github.com/benoitvidis))

#### Enhancements

- [ [#439](#439) ] Add timeout support to Http protocol   ([Aschen](https://github.com/Aschen))
---
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants