-
Notifications
You must be signed in to change notification settings - Fork 11
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
List scoped common variables #488
Conversation
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've tested with feature toggles on and off. The correct variables are shown in both cases.
I think the choice to print a row per environment is a good one given our constraints. We can iterate on the design if feedback indicates that it is confusing.
I have a suspicion that we may end up with the wrong answer when asking if the feature toggle is disabled for versions of Octopus that do not have the feature toggle filter implemented. We may need to get a name match (and handle when the name does not exist).
} | ||
var allVariableValues []*VariableValue | ||
for _, element := range vars.LibraryVariables { | ||
if toggleValue { |
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 was going to recommend splitting this into two functions, one for toggle on and off. But perhaps this makes more sense if we are going to remove one half anyway.
returnToggleResponse, err := configuration.Get(client, toggleRequest) | ||
|
||
if err != nil { | ||
return false, err |
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.
What happens when the toggle name parameter does not exist? ie the cli is being run against a version of Octopus that doesn't have the toggle filter implemented.
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.
Good catch. I've updated the function to handle an empty list of toggles, as well as a name check since older versions of octopus return all toggles instead of just one
[SC-93474]
When scoped tenant common variables are enabled, both common and project variables can be scoped by multiple environments. This PR provides support for variables with an array of environment scopes.
Before
Tenant common variables had no environment scoping, so Environment was blank. Project variables were only scoped to one environment, so per template, one row existed per environment.
After
Tenant common variables now have environment scoping, and both common and project variables can have multiple environment scopes. We still list one row per template and environment because each environment can only have one value. Tested combining environment names for shared scopes into a comma separated list, but values were cut off when the list was long.