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

feat(webex-core): adding support for a custom proxy #3624

Open
wants to merge 3 commits into
base: next
Choose a base branch
from

Conversation

Tiuipuv
Copy link

@Tiuipuv Tiuipuv commented May 29, 2024

This pull request addresses

Adding support for an app to set a custom proxy on all requests. The app can control this by setting the proxy configuration during init. e.g.

webex = WebexSdk.init({
  credentials: {
    supertoken: superToken
  },
  config: {
    credentials: {
      client_id,
      client_secret
    },
    proxy: 'http://proxy.company.com'
  }
});

By allowing for custom proxy it'll be easier for organizations utilizing a proxy to be able to connect to the webex sdk via NodeJS.

by making the following changes

These changes include:

  • Adding a new interceptor class for webex-core, called ProxyInterceptor
  • Adding the interceptor into webex-core, to run against every onRequest
  • Adding new unit test file for ProxyInterceptor

Change Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Tooling change
  • Internal code refactor

The following scenarios where tested

  • no proxy given (automated)
  • string proxy is given in webex init config (automated)

I certified that

  • I have read and followed contributing guidelines
  • I discussed changes with code owners prior to submitting this pull request
  • I have not skipped any automated checks
  • All existing and new tests passed
  • I have updated the documentation accordingly

Make sure to have followed the contributing guidelines before submitting.

@Tiuipuv Tiuipuv requested a review from a team as a code owner May 29, 2024 20:47
@Tiuipuv
Copy link
Author

Tiuipuv commented May 29, 2024

If this PR is accepted, it is recommended that the proxy documentation on this wiki page be updated to include both the websocket proxy setup as well as this new http proxy setup.

@Tiuipuv Tiuipuv mentioned this pull request Jun 10, 2024
@sreenara sreenara added the validated If the pull request is validated for automation. label Jun 10, 2024
@sreenara sreenara requested review from sreenara and a team June 10, 2024 17:05
@sreenara
Copy link
Contributor

@Tiuipuv, the SDK team will review this PR. Please stand-by.

@mkesavan13
Copy link
Contributor

Hi @Tiuipuv

Sorry for the delay. I am looking into this PR. While I do that, could you please get the latest changes from the next branch updated to your branch?

@mkesavan13
Copy link
Contributor

Also, I see Integration test failures. Please look into it. @Tiuipuv

@Tiuipuv Tiuipuv force-pushed the feat/core/add-proxy-for-node branch 2 times, most recently from 7befb70 to 76e6869 Compare August 2, 2024 19:11
@Tiuipuv
Copy link
Author

Tiuipuv commented Aug 2, 2024

PR is updated to latest next branch. I will investigate the integration failure next week. I do not have access to the 4 environment variables from my organization:

WEBEX_CLIENT_ID=""
WEBEX_CLIENT_SECRET=""
WEBEX_APPID_ORGID=""
WEBEX_APPID_SECRET=""  

is there any shared test environment I can use?

@Tiuipuv Tiuipuv force-pushed the feat/core/add-proxy-for-node branch from 76e6869 to 1913f20 Compare August 7, 2024 14:33
@Tiuipuv Tiuipuv force-pushed the feat/core/add-proxy-for-node branch from 1913f20 to 112c99a Compare August 7, 2024 16:18
@sreenara
Copy link
Contributor

sreenara commented Aug 7, 2024

PR is updated to latest next branch. I will investigate the integration failure next week. I do not have access to the 4 environment variables from my organization:

WEBEX_CLIENT_ID=""
WEBEX_CLIENT_SECRET=""
WEBEX_APPID_ORGID=""
WEBEX_APPID_SECRET=""  

is there any shared test environment I can use?

There isn't any shared test environment unfortunately. @mkesavan13 we will need to take a look at this and see if this is an existing failure.

@mkesavan13 mkesavan13 removed the validated If the pull request is validated for automation. label Aug 8, 2024
@Tiuipuv
Copy link
Author

Tiuipuv commented Sep 30, 2024

Thanks for the info @sreenara. Let me know if there is anything I can do to help with this Pull Request @mkesavan13!

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