Skip to content

Conversation

@DevOpsDave
Copy link
Contributor

@DevOpsDave DevOpsDave commented Jan 9, 2025

Change Log Items

  • Remove alks-node dependency.
  • Remove node-fetch/request in favor for axios

Description

Because prior to node 18, node did not provide an http fetch api so we had to use a 3rd party library for that functionality (request/node-fetch). These 3rd party packages rely on punycode which is being deprecated in node >= v21. This PR will switch over to axios since that will relieve us of the punycode deprecation warnings.

Rally:

US1504256: [Continued] [Continued] alks-cli: Clean deprecation warnings

@DevOpsDave DevOpsDave added the release/patch Indicates an update without breaking changes or new features label Jan 9, 2025
@DevOpsDave DevOpsDave marked this pull request as ready for review January 13, 2025 13:01
Copy link
Contributor

@americk0 americk0 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. The only issue I discovered was with the alks server start command not working, but to be fair I installed an older version of the ALKS cli from before your changes and it still gave me these errors so it looks like this was already broken. Rather than bothering to fix this (since it's legacy and not working currently), maybe we should just remove it in the near future from the ALKS CLI altogether, and if a few people request if back maybe we can fork it into a separate project. Just something to consider, but for now this code looks and runs great as far as I've tested

Screenshot 2025-01-13 at 11 11 31 AM

@DevOpsDave DevOpsDave merged commit 03a6b12 into master Jan 13, 2025
4 checks passed
@DevOpsDave DevOpsDave deleted the fix_deprecation_warnings branch January 13, 2025 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release/patch Indicates an update without breaking changes or new features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants