Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FE-2750][FE-2754] Support connection pool and network level timeouts and maxConns in ClientConfiguration #3

Merged
merged 2 commits into from
Oct 12, 2022

Conversation

cleve-fauna
Copy link
Contributor

@cleve-fauna cleve-fauna commented Oct 10, 2022

Ticket(s): FE-2750, FE-2754

Problem

  1. Need to support connection pooling.
  2. Need to support network level timeouts.
  3. Need a way to run integ tests

Solution

  1. Utilize agent-keep-alive to build a connection pool and enforce network level timeouts. Agent-keep-alive fixes an outstanding bug in NodeJS's underlying HTTP Agent where IP changes on the server are not well handled. See here.
  2. Set timeout for network issues.
  3. Reconfigured the tests to optionally run as integ tests against a "real" endpoint.

Result

Client pool settings and such available to the user.

Out of scope

Vending a default client configuration.

Testing

Tested locally:

~/workplace/fql-x-javascript (main_sp_wp_clientConfig) » yarn test                                                                                                                                                                                                                                                                                                      
 $ jest
 PASS  __tests__/functional/client-configuration.test.ts
 PASS  __tests__/integration/query.test.ts

Test Suites: 2 passed, 2 total
Tests:       3 passed, 3 total
Snapshots:   0 total
Time:        2.663 s
Ran all test suites.
 ✨  Done in 3.90s.

and against preview:

~/workplace/fql-x-javascript (main_sp_wp_clientConfig) » endpoint="https://db.prev.faunadb.net" secret=REDACTED yarn test                                                                                                                                                                                                                cleve@Cleves-MBP
 $ jest
 PASS  __tests__/functional/client-configuration.test.ts
 PASS  __tests__/integration/query.test.ts

Test Suites: 2 passed, 2 total
Tests:       3 passed, 3 total
Snapshots:   0 total
Time:        2.558 s, estimated 3 s
Ran all test suites.
 ✨  Done in 3.10s.

@cleve-fauna
Copy link
Contributor Author

Please do not merge until #1 and #2 are merged.

Copy link
Member

@freels freels left a comment

Choose a reason for hiding this comment

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

LGTM, though I think it would make sense to ask for at least on DX person to take a look at these, since with the exception of Jake, I'm not sure we can give you as valuable feedback on the JS aspect of the code.

src/client.ts Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants