Include proxy support#28
Conversation
Codecov Report
@@ Coverage Diff @@
## master #28 +/- ##
==========================================
- Coverage 93.51% 93.33% -0.18%
==========================================
Files 14 14
Lines 447 450 +3
Branches 45 46 +1
==========================================
+ Hits 418 420 +2
- Misses 29 30 +1
Continue to review full report at Codecov.
|
jperata
left a comment
jperata
left a comment
There was a problem hiding this comment.
Great feedback on how to use it with proxy, but current implementation it's difficult to test and to add to different libraries.
Would it be possible for you to add the changes and a test for this.
If not i will make the updates on this PR as soon as i finish on a different change (#22) that's on the way.
|
|
||
| ### Proxy settings | ||
|
|
||
| If you need to use a custom proxy, just set your `HTTP_PROXY` environment variable. |
There was a problem hiding this comment.
To use and env variable makes it difficult to use this library as part of other solutions, for example we plan on adding it to bst tools so that you can use similar commands to alexa.
It should be added as part of the VirtualGoogleAssistantBuilder
|
|
||
| export class Invoker { | ||
| public static async invokeExpressFile(fileName: string, port: number, jsonRequest: any, restOfPath?:string): Promise<any> { | ||
| public static async invokeExpressFile(fileName: string, port: number, jsonRequest: any, restOfPath?: string): Promise<any> { |
There was a problem hiding this comment.
this should include an extra parameter for the proxy.
| port: url.port ? parseInt(url.port, 10) : undefined, | ||
| }; | ||
|
|
||
| if(process.env.http_proxy){ |
There was a problem hiding this comment.
should come from the constructor instead of env
2 participants
Our workspace required us to go through proxy authentication to hit an outside server (which is where our code is stored). This PR is what allowed us to use this package through the proxy.