Make changes to support connection timeouts and hostname validation#2486
Make changes to support connection timeouts and hostname validation#2486
Conversation
Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
awharn
requested review from
ATorrise,
anaxceron,
gejohnston,
jace-roell,
pujal0909,
t1m0thyj,
traeok and
zFernand0
|
📅 Suggested merge-by date: 4/29/2025 |
Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2486 +/- ##
========================================
Coverage 91.55% 91.56%
========================================
Files 641 641
Lines 18480 18498 +18
Branches 3918 4036 +118
========================================
+ Hits 16920 16938 +18
Misses 1558 1558
Partials 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| import { ZosFilesUtils } from "../../utils/ZosFilesUtils"; | ||
|
|
||
| export class Utilities { | ||
| private static readonly minimumTimeout = 5; |
There was a problem hiding this comment.
should this timeout be added as a constant? in Constants.ts? only because a min timeout of 5 is used a lot of places
There was a problem hiding this comment.
I only did this here because of the several uses in this one file. The only other locations where I could find 5 being used was in the zosfiles strings file, and as a parameter for responseTimeout in the zosmf package. The only places where the change could be placed to share that value across those three files is either zosmf or core, and a new constants object would need to be created to export it. Seeing as the property here is readonly, I plan to leave the implementation as-is until we find more constants that we would like to expose at a lower level.
github-project-automation
Bot
moved this from In Progress
to Review/QA
in Zowe CLI Squad
anaxceron
left a comment
anaxceron
left a comment
There was a problem hiding this comment.
Left a comment re: the changelog
Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
|
traeok
left a comment
traeok
left a comment
There was a problem hiding this comment.
Tested and LGTM, thanks Andrew!
zFernand0
left a comment
zFernand0
left a comment
There was a problem hiding this comment.
LGTM! 😋
Pardon the late review 😅
Thanks for adding this as an ENV 🥳
zFernand0
added
the
release-current
|
tagging release current to combine with the 2 other open PRs 🙏 |
8 participants
What It Does
Adds a connection timeout to the Abstract Rest Client.
Adds hostname validation to REST requests.
Fixes linter errors.
Makes imports more granular to prevent circular imports.
How to Test
Review Checklist
I certify that I have:
Additional Comments