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

Updated deprecated dependency request-promise-native #1163

Merged

Conversation

yashkohli88
Copy link
Contributor

@yashkohli88 yashkohli88 commented Jul 29, 2024

The service code has been updated to replace the deprecated dependency, request-promise-native, with axios. The following tasks have been performed:

  • A fetch.js file was created in the lib folder. This file contains a function which takes request parameters as input, uses the axios library to initiate HTTP requests, and returns responses. It accepts method, URL, responseType ('json', 'text', 'stream'), headers, and data as request parameters.
  • This function was integrated into the files of various folder wherever the request-promise-native library was previously invoked. Various files and functions were updated to accommodate this change.
  • All replaced statements were individually tested, and an integration test suite was also used to ensure that no functionality was broken due to these changes.
  • Test files were updated where stubs had been declared using request-promise-native.
  • The request-promise-native library was removed from the package.json file.
  • A test file for fetch.js, named fetchTests.js, was created to test the function with various scenarios. Please note that an internet connection is necessary to run these test cases.

Future scope -

  • Refactor the code to declare the headers in centralised fetch.js file

This PR is related to the issue #1090

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@qtomlinson I request your help to test all the requestPromise calls in this file.

Copy link
Collaborator

@qtomlinson qtomlinson Aug 23, 2024

Choose a reason for hiding this comment

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

tested against dev instance. Also adding integration test for this. See clearlydefined/operations#89

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@qtomlinson I request your help to test all the requestPromise calls in this file.

Copy link
Collaborator

Choose a reason for hiding this comment

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

tested against dev instance. Also adding integration test for this. See clearlydefined/operations#89

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@qtomlinson I request your help to test the requestPromise call(_fetch) in this file.

Copy link
Collaborator

@qtomlinson qtomlinson Aug 23, 2024

Choose a reason for hiding this comment

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

tested with pypi/pypi/-/backports.ssl-match_hostname/3.7.0.1

@qtomlinson qtomlinson marked this pull request as draft July 30, 2024 17:55
Add relevant tests. Remove the integration tests to download crate components
@yashkohli88 yashkohli88 force-pushed the yk/update-request-promise branch from d9b7a1d to cec14d2 Compare September 19, 2024 14:56
@yashkohli88 yashkohli88 force-pushed the yk/update-request-promise branch from cec14d2 to f99cf6b Compare September 24, 2024 08:38
@yashkohli88 yashkohli88 force-pushed the yk/update-request-promise branch from f99cf6b to b2895db Compare September 24, 2024 09:46
@yashkohli88 yashkohli88 marked this pull request as ready for review September 25, 2024 13:22
Copy link
Collaborator

@elrayle elrayle left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for taking care of this technical debt.

@elrayle elrayle merged commit f0e4562 into clearlydefined:master Oct 2, 2024
4 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