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
498 http support #909
498 http support #909
Conversation
Signed-off-by: AndrewTwydell <u1858490@unimail.hud.ac.uk>
Signed-off-by: AndrewTwydell <u1858490@unimail.hud.ac.uk>
Signed-off-by: AndrewTwydell <u1858490@unimail.hud.ac.uk>
Signed-off-by: AndrewTwydell <u1858490@unimail.hud.ac.uk>
Signed-off-by: AndrewTwydell <u1858490@unimail.hud.ac.uk>
Signed-off-by: AndrewTwydell <u1858490@unimail.hud.ac.uk>
Do I need to update the Changelog in the Zosmf package for this PR? I've updated the CLI package Changelog as a new flag has been added, but the unchanged zosmf changelog is preventing the build from being successful. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
I know this probably doesn't concern this PR, but should we try to expose the protocol in the Imperative AbstractRestClient Error messages (now that it's available)?
See zowe/imperative#539 for updates on the AbstractRestClient error details.
…tp-support Signed-off-by: zFernand0 <fernando.rijocedeno@broadcom.com> # Conflicts: # packages/cli/CHANGELOG.md
Signed-off-by: zFernand0 <fernando.rijocedeno@broadcom.com>
Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
packages/zosmf/src/ZosmfSession.ts
Outdated
public static ZOSMF_OPTION_PROTOCOL: ICommandOptionDefinition = { | ||
name: "protocol", | ||
aliases: ["o"], | ||
description: "The protocol used (HTTP or HTTPS)", | ||
type: "string", | ||
group: ZosmfSession.ZOSMF_CONNECTION_OPTION_GROUP | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should "http" and "https" be defined as the only allowed values for the --protocol command option?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see a problem in doing so. 😋
Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🙂
Forgot when I reviewed earlier that CICS plugin also has defaultValue: "https"
defined on the command option definition for protocol 😢
Perhaps we should do the same, but I'm ok with this being merged as is - the defaultValue
property doesn't seem important since the behavior behind the scenes is the same (HTTPS is the default for ZosmfSession).
Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for adding defaultValue
and good catch on the potential breaking change 😛
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@@ -156,7 +171,8 @@ export class ZosmfSession { | |||
user: profile.user, | |||
password: profile.password, | |||
rejectUnauthorized: profile.rejectUnauthorized, | |||
basePath: profile.basePath | |||
basePath: profile.basePath, | |||
protocol: profile.protocol ? profile.protocol.toLowerCase() : 'https' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for preventing any strange behavior with the old method 😋
Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
SonarCloud Quality Gate failed. |
Resolves #498
This PR implements a
--protocol
or-o
flag to specify eitherhttp
orhttps
as a protocol. The default is stillhttps
.Imperative already supports a session including a protocol option and defaults it to https if not supplied. This PR sets the protocol parameter when the ISession is created.