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

chore: get rid of deprecated request-promise and request #4898

Closed
Walther opened this issue Jul 28, 2023 · 0 comments · Fixed by #5233
Closed

chore: get rid of deprecated request-promise and request #4898

Walther opened this issue Jul 28, 2023 · 0 comments · Fixed by #5233
Labels

Comments

@Walther
Copy link
Contributor

Walther commented Jul 28, 2023

request and request-promise have been deprecated in 2020:

As of Feb 11th 2020, request is fully deprecated. No new changes are expected to land. In fact, none have landed for some time. This package is also deprecated because it depends on request.
https://github.com/request/request-promise

We should remove these dependencies fully from our dependency tree.


We should not be migrating to axios, as that dependency has actually been removed recently: #3614

We have an internal re-export of the got library in core/src/util/http.ts, which should probably be used instead:

// Exporting from here to make sure the global-agent bootstrap is executed, and for convenience as well
export const got = _got


This task is blocked by the upstream kubernetes client library we use, as it is still using request (and expects its option types as params):

@Walther Walther added the chore label Jul 28, 2023
stefreak added a commit that referenced this issue Nov 6, 2023
This had many different effects that derived from it, making this PR so massive.

**Modules**:
The first stage was to rewrite all the imports to use .js endings everywhere.
Then we needed to replace and bump a bunch of dependencies to their ESM version.

**Bundling**:
Following that we tried to use rollup to bundle everything back into CommonJS so that we can use it in pkg since pkg does not support ESM.
This was unsuccessful, and after trying several different single application solutions, we decided we will have to write our own.

**Single Executable Binary**:
We now use a custom Rust binary which bundles a zipped version of NodeJS, the native extensions and the bundled code.
We bundle the code for tree-shaking and size reduction, but also because otherwise we had issues with how we ensure that native modules are loaded correctly and with resolving the imports correctly cross-platform.
Then on first run, the binary extracts those files to the filesystem and runs node from there.
There are checksum files that we bundle together with the archives and that we store on disk to see if we need to re-extract or not.
This also gives us a huge performance boost in some cases - especially when lots of filesystem reads are required - since pkg was patching and proxying those methods and making things slow.

**Kubernetes client**:
We also had to update the Kubernetes library to the 1.0.0-rc3, with a patch from a fork that contains the changes for kubernetes-client/javascript#1341
That is because the library was using request-promise behind the scenes, which started showing up with unhandled rejection warnings and errors about req not being defined in some cases.
That library update required us to change almost every k8s call since the interface now uses objects instead of positional arguments, which makes things much nicer to read and use. We also had to introduce a workaround for using custom certificates together with proxies, something that was previously globally monkey-patched by global-agent.

**request-promise**:
We no longer depend on request-promise and fully removed the dependency.

Fixes #3841
Fixes #4898
github-merge-queue bot pushed a commit that referenced this issue Nov 6, 2023
This had many different effects that derived from it, making this PR so massive.

**Modules**:
The first stage was to rewrite all the imports to use .js endings everywhere.
Then we needed to replace and bump a bunch of dependencies to their ESM version.

**Bundling**:
Following that we tried to use rollup to bundle everything back into CommonJS so that we can use it in pkg since pkg does not support ESM.
This was unsuccessful, and after trying several different single application solutions, we decided we will have to write our own.

**Single Executable Binary**:
We now use a custom Rust binary which bundles a zipped version of NodeJS, the native extensions and the bundled code.
We bundle the code for tree-shaking and size reduction, but also because otherwise we had issues with how we ensure that native modules are loaded correctly and with resolving the imports correctly cross-platform.
Then on first run, the binary extracts those files to the filesystem and runs node from there.
There are checksum files that we bundle together with the archives and that we store on disk to see if we need to re-extract or not.
This also gives us a huge performance boost in some cases - especially when lots of filesystem reads are required - since pkg was patching and proxying those methods and making things slow.

**Kubernetes client**:
We also had to update the Kubernetes library to the 1.0.0-rc3, with a patch from a fork that contains the changes for kubernetes-client/javascript#1341
That is because the library was using request-promise behind the scenes, which started showing up with unhandled rejection warnings and errors about req not being defined in some cases.
That library update required us to change almost every k8s call since the interface now uses objects instead of positional arguments, which makes things much nicer to read and use. We also had to introduce a workaround for using custom certificates together with proxies, something that was previously globally monkey-patched by global-agent.

**request-promise**:
We no longer depend on request-promise and fully removed the dependency.

Fixes #3841
Fixes #4898

Co-authored-by: Steffen Neubauer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant