Skip to content
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

Introduce v3 command names for run-action and run #10563

Merged
merged 1 commit into from Aug 27, 2019

Conversation

wallyworld
Copy link
Member

Description of change

Add a "juju-v3" feaure flag and start introducing new commands for run-action and run.
juju-action -> juju run
juju run -> juju exec

With flag not enabled, juju run is aliased to juju exec

QA steps

export JUJU_DEV_FEATURE_FLAGS=juju-v3
juju run ....
juju exec ...

@wallyworld
Copy link
Member Author

Name: "run-action",
Args: "<unit> [<unit> ...] <action name> [key.key.key...=value]",
Purpose: "Queue an action for execution.",
Doc: runDoc,
})
if featureflag.Enabled(feature.JujuV3) {
info.Name = "run"
info.Doc = strings.Replace(info.Doc, "run-action", "run", -1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to append a note to the doc like "NOTE: pre-3.0 juju run functionality is provided by juju exec"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't thin we need to do this because this v3 behaviour is strictly opt-in - the user has to enable a feature flag to see it. They are therefore already aware of what they are doing in that sense and what the command names are etc.

@@ -246,8 +250,8 @@ func ConvertActionResults(result params.ActionResult, query actionQuery) map[str
return values
}

func (c *runCommand) Run(ctx *cmd.Context) error {
client, err := getRunAPIClient(c)
func (c *execCommand) Run(ctx *cmd.Context) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't look possible with juju/cmd, but ideally we should be able to warn when a deprecated alias is used. Something like "WARN: juju exec will soon supersede juju run".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, not really possible. We'll highlight in the juju 3 release notes

@wallyworld
Copy link
Member Author

$$merge$$

1 similar comment
@wallyworld
Copy link
Member Author

$$merge$$

@jujubot jujubot merged commit 0e1db37 into juju:develop Aug 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants