-
Notifications
You must be signed in to change notification settings - Fork 25
fix(shell): prevent crash when target namespace lacks IPM mappings #994
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
cfd9377
bd26d94
b3152b5
c871ba8
6ade838
887d594
df07f0b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1076,7 +1076,11 @@ ClassMethod Namespace(ByRef pCommandInfo) [ Internal ] | |
| set name = $translate($get(pCommandInfo("parameters","name"),"*"),$char(34)) | ||
| if (name'["*") { | ||
| try { | ||
| set $namespace = name | ||
| if '..IsIPMEnabled(name) { | ||
| write $$$FormattedLine($$$Red," IPM is not enabled in this namespace.") | ||
| } else { | ||
| set $namespace = name | ||
| } | ||
| } catch ex { | ||
| $$$ThrowStatus($$$ERROR($$$GeneralError,"Failed to switch to namespace: " _ name)) | ||
| } | ||
|
|
@@ -1099,7 +1103,14 @@ ClassMethod Namespace(ByRef pCommandInfo) [ Internal ] | |
| do ##class(%Library.Prompt).GetString(prompt, .value) | ||
| quit:value="" | ||
| try { | ||
| set $namespace = $select($data(list(value), ns): $listget(ns), 1: value) | ||
| set selectedNamespace = $select($data(list(value), ns): $listget(ns), 1: value) | ||
| if '..IsIPMEnabled(selectedNamespace) { | ||
| write $$$FormattedLine($$$Red," IPM is not enabled in this namespace.") | ||
|
||
| hang 0.5 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the purpose of the hang? Its generally not great practice to have hangs (exceptions are for polling purposes)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @isc-kiyer
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @AshokThangavel Ah right. I think it may be a better user experience to accumulate all namespaces where IPM is not enabled and print it at the end instead of them needing to hunt for lines in between possibly long output.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for the feedback. However, the namespace, zn command already displays the ZPM-enabled details. The crash occurs only when we attempt to select a namespace where IPM is not enabled. zpm:USER>zn
1. %SYS>
2. USER> zpm 0.10.5-SNAPSHOT
testify 1.0.2
web-fslog 1.0.2
objectscript-math 0.0.5
3. USER-VERIFY> zpm 0.10.5-SNAPSHOT
4. USER-VERIFY-HISTORY> simplest-module 1.0.0
5. USER-VERIFY-VERIFY> testmodule 1.0.0As you suggested, Can we display this existing message at the end by default %SYS>zpm
IPM is not enabled in this namespace.
Change namespace to one of the following to run the "zpm" command
USER> zpm 0.10.5-SNAPSHOT
USER-VERIFY> zpm 0.10.5-SNAPSHOT
USER-VERIFY-HISTORY> zpm 0.10.5-SNAPSHOT
Installed In: USER-VERIFY
USER-VERIFY-VERIFY> zpm 0.10.5-SNAPSHOT
Installed In: USER-VERIFY
https://pm.community.intersystems.com/ - 1.0.6
If you want to map IPM globally, switch to one of the namespaces above and run: zpm "enable -map -globally".
If you want to reset repository and map IPM globally along with repository settings, switch to one of the namespaces above and run: zpm "enable -community".or simple summary of namespace and whether IPM enabled or not 1. %SYS> IPM not enabled
2. USER> IPM enabled
.... |
||
| write $char(13),*27,"[K",*27,"[A" | ||
|
||
| continue | ||
| } | ||
| set $namespace = selectedNamespace | ||
| } catch e { | ||
| write $char(13),*27,"[K",*27,"[A" | ||
| continue | ||
|
|
@@ -3845,7 +3856,8 @@ ClassMethod DisplayModules( | |
| if pNumbered { | ||
| write $justify(i, numbersWidth - 2), ". " | ||
| } | ||
| write $$$FormattedLinePadRight($$$Magenta, tNS _ "> ", nsWidth) | ||
| set ipmStatus=$select('..IsIPMEnabled(tNS):"(IPM not enabled)",1:"") | ||
| write $$$FormattedLinePadRight($$$Magenta, tNS _ "> "_ipmStatus, nsWidth) | ||
| set tPrefix = $justify("", numbersWidth) _ tNS | ||
| kill tModulesList | ||
| merge tModulesList = pList(i, "modules") | ||
|
|
@@ -3924,6 +3936,13 @@ ClassMethod DisplayModules( | |
| } | ||
| } | ||
|
|
||
| ClassMethod IsIPMEnabled(pNamespace As %String = {$namespace}) As %Boolean | ||
| { | ||
| new $namespace | ||
| set $namespace = pNamespace | ||
| return $system.CLS.IsMthd("%IPM.Main", "Shell") | ||
| } | ||
|
|
||
| ClassMethod GetUpstreamPackageVersions(Output list) | ||
| { | ||
| set query = "SELECT %DLIST(Name) AS Repos FROM %IPM_Repo.Definition" | ||
|
|
||
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.
Include
namein the messageThere 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.
Included the name in the message!