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

js: DriverRemoteConnection: allow custom agent #2240

Merged

Conversation

itsjfx
Copy link
Contributor

@itsjfx itsjfx commented Sep 15, 2023

Allow users to provide their own http.Agent when initialising DriverRemoteConnection.

This is extremely helpful connecting to databases within a corporate network.
This is utilising the agent support provided by the ws library

Usage with a SOCKS proxy with socks-proxy-agent:

const { SocksProxyAgent } = require('socks-proxy-agent');
const agent = new SocksProxyAgent(
    'socks://127.0.0.1:1080'
);
const dc = new DriverRemoteConnection('wss://xx:8182/gremlin', { agent });

@codecov-commenter
Copy link

codecov-commenter commented Sep 15, 2023

Codecov Report

Merging #2240 (599f3b7) into master (2bcdb66) will decrease coverage by 0.32%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #2240      +/-   ##
============================================
- Coverage     76.11%   75.79%   -0.32%     
+ Complexity    12715    12706       -9     
============================================
  Files          1035     1060      +25     
  Lines         60440    64335    +3895     
  Branches       7105     7105              
============================================
+ Hits          46002    48761    +2759     
- Misses        11989    12926     +937     
- Partials       2449     2648     +199     

see 32 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@itsjfx
Copy link
Contributor Author

itsjfx commented Sep 17, 2023

Hi @spmallette sorry to bother you directly. Could you review my PR please :)

@vkagamlyk
Copy link
Contributor

Hi @itsjfx,

Thanks for your contribution, in general I understand why this is needed.
a couple of small questions:

  • in which version do you need this change? in 3.5, 3.6 or 3.7?
  • it would be nice to add a line with a description to the changelog

Allow users to provide their own http.Agent when initialising
DriverRemoteConnection.

This is extremely helpful connecting to databases within a corporate
network.
@itsjfx itsjfx force-pushed the allow-agent-in-driver-remote-connection branch from 7e32025 to 5ed6a5b Compare September 21, 2023 06:43
@itsjfx
Copy link
Contributor Author

itsjfx commented Sep 21, 2023

hi @vkagamlyk ,
I'm happy with my changes to be rolled into 3.7. Have updated the changelog to reflect this as you requested.

@vkagamlyk
Copy link
Contributor

thank you @itsjfx,

VOTE+1

to merge this PR need 3 votes or lazy consensus (simply wait a week)

@vkagamlyk vkagamlyk merged commit ca5b9c8 into apache:master Sep 28, 2023
24 checks passed
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