add keepalive_timeout flag#6674
Conversation
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
|
Please run |
|
@mjjbell Thanks for your comment, I have fixed the format problem, could you rerun the workflow? |
fenwuyaoji
force-pushed
the
keepalive_timeout-flag
branch
from
c3a8e5e to
31c044a
Compare
|
@mjjbell Sorry for the late response. I checked the page https://github.com/Project-OSRM/osrm-backend/blob/master/CONTRIBUTING.md and added |
|
@mjjbell Sorry to disturb you again. I saw there was one test case that didn't pass. I'm not familiar with the js test so I just copied and modified another one as mine. Now I notice that, if I want to throw an error, I should first define it in node_osrm_support.hpp. I checked the case and thought it unnecessary so I deleted it. Could you rerun it? |
|
@mjjbell Really appreciate! Have a good day~ |
test/nodejs/index.js
Outdated
|
|
||
| test('constructor: takes a keepalive_timeout argument', function(assert) { | ||
| assert.plan(1); | ||
| var osrm = new OSRM({algorithm: 'MLD', path: monaco_mld_path, keepalive_timeout: 10}); |
There was a problem hiding this comment.
The Node bindings do not use the HTTP server, so the keepalive_timeout won't do anything here.
I would just remove the test.
There was a problem hiding this comment.
Sure, I just remove it.
* add keepalive_timeout flag
2 participants
Issue
[What issue is this PR targeting? If there is no issue that addresses the problem, please open a corresponding issue and link it here.]
Relevant to:
#6673 Rquire a configurable parameter to set keepalive_timeout.
Please read our documentation on release and version management.
If your PR is still work in progress please attach the relevant label.
Tasklist
Requirements / Relations
Link any requirements here. Other pull requests this PR is based on?