feat(environments): Preflight cli changes #246

anna-cross · 2022-02-07T21:42:57Z

Description of change

We want to be able to see preflight check information on cli when creating an environment. On 'meroxa env describe' we will lis t full environment information with preflight checks. When creating an environment, we want to log to console if preflight check is successful or errored out.

Fixes #245

Type of change

New feature
Bug fix
Refactor
Documentation

How was this tested?

Unit Tests
Tested in staging

Demo

Before the Changes:

Creating bad environment with not enough permissions

Describing bad environment that failed to provision

After the Changes :

Creating bad environment with not enough permissions

Describing bad environment that failed to provision

Repairing environment after giving necessary permissions

Additional references

Documentation updated

N/A

remove unnecessary attribute assignments

…into preflight_cli_changes

dianadoherty · 2022-02-08T17:08:14Z

can you add before and after exampled for each command?

janelletavares · 2022-02-08T19:37:23Z

cmd/meroxa/root/environments/create.go

@@ -54,7 +53,7 @@ type Create struct {
 		Type     string   `long:"type" usage:"environment type, when not specified"`
 		Provider string   `long:"provider" usage:"environment cloud provider to use"`
 		Region   string   `long:"region" usage:"environment region"`
-		Config   []string `short:"c" long:"config" usage:"environment configuration based on type and provider (e.g.: --config aws_access_key_id=my_access_key --config aws_access_secret=my_access_secret)"` // nolint:lll
+		Config   []string `short:"c" long:"config" usage:"environment configuration based on type and provider (e.g.: --config aws_access_key_id=my_access_key --config aws_secret_access_key=my_access_secret)"` // nolint:lll


good catch!

janelletavares

i'm biased, but i think these changes are looking pretty good

anna-cross · 2022-02-08T22:55:14Z

can you add before and after exampled for each command?

Added! Let me know if we are good to merge this or anything else needs to be updated :)

dianadoherty · 2022-02-09T16:42:38Z

There seems to be an inconsistency in spacing Describing bad environment that failed to provision and Repairing environment after giving necessary permissions

I'd also prefer for the output of Creating bad environment with not enough permissions to be in the same format as the describe

dianadoherty · 2022-02-09T22:12:33Z

cmd/meroxa/root/environments/repair.go

+		text := fmt.Sprintf("Environment %q could not be repaired because it failed the preflight checks.", name)
+		details := utils.EnvironmentPreflightTable(rr)
+		text += fmt.Sprintf("\n%s\n", details)
+
+		r.logger.Errorf(ctx, text)


I recommend staying consistent between all operations. This is an operation code taken from create that seems a little neater

Suggested change

text := fmt.Sprintf("Environment %q could not be repaired because it failed the preflight checks.", name)

details := utils.EnvironmentPreflightTable(rr)

text += fmt.Sprintf("\n%s\n", details)

r.logger.Errorf(ctx, text)

details := utils.EnvironmentPreflightTable(rr)

c.logger.Errorf(ctx,

"Environment %q could not be repaired because it failed the preflight checks\n%s\n",

environment.Name,

details)

dianadoherty · 2022-02-09T22:16:05Z

cmd/meroxa/root/environments/create.go

+			environment.Name,
+			details)
+	} else {
+		environment.Status.PreflightDetails = nil


I don't think it's necessary to clear out preflight check details. Customers might want to see this info in the --json command

Suggested change

environment.Status.PreflightDetails = nil

dianadoherty · 2022-02-09T22:17:15Z

cmd/meroxa/root/environments/repair.go

+	} else if state == meroxa.EnvironmentStateRepairing || state == meroxa.EnvironmentStatePreflightSuccess {
+		r.logger.Infof(ctx, `The repairment of your environment %q is now in progress and your environment will be up and running soon.`, r.args.NameOrUUID) // nolint:lll
+	} else if state == meroxa.EnvironmentStateRepairingError || state == meroxa.EnvironmentStateUpdatingError ||
+		state == meroxa.EnvironmentStateProvisioningError || state == meroxa.EnvironmentStateDeprovisioningError {
+		text := fmt.Sprintf("Environment %q could not be repaired.", r.args.NameOrUUID)
+		if details, err := utils.PrettyString(rr.Status.Details); err == nil {
+			text += fmt.Sprintf("\n%s\n", details)
+		}
+		r.logger.Infof(ctx, text)
+	}
 	r.logger.Infof(ctx, `Run "meroxa env describe %s" for status.`, r.args.NameOrUUID)


Suggested change

} else if state == meroxa.EnvironmentStateRepairing || state == meroxa.EnvironmentStatePreflightSuccess {

r.logger.Infof(ctx, `The repairment of your environment %q is now in progress and your environment will be up and running soon.`, r.args.NameOrUUID) // nolint:lll

} else if state == meroxa.EnvironmentStateRepairingError || state == meroxa.EnvironmentStateUpdatingError ||

state == meroxa.EnvironmentStateProvisioningError || state == meroxa.EnvironmentStateDeprovisioningError {

text := fmt.Sprintf("Environment %q could not be repaired.", r.args.NameOrUUID)

if details, err := utils.PrettyString(rr.Status.Details); err == nil {

text += fmt.Sprintf("\n%s\n", details)

}

r.logger.Infof(ctx, text)

}

r.logger.Infof(ctx, `Run "meroxa env describe %s" for status.`, r.args.NameOrUUID)

} else {

c.logger.Infof(ctx,

"Preflight checks have passed. Environment %q is being repaired. Run `meroxa env describe %s` for status",

environment.Name,

environment.Name)

}

dianadoherty · 2022-02-09T22:18:46Z

cmd/meroxa/root/environments/update.go

+		text := fmt.Sprintf("Environment %q could not be updated.", c.args.NameOrUUID)
+		details := utils.EnvironmentPreflightTable(environment)
+		text += fmt.Sprintf("\n%s\n", details)
+		c.logger.Infof(ctx, text)
+	} else if state == meroxa.EnvironmentStatePreflightError {
+		text := fmt.Sprintf("Environment %q could not be updated because it failed the preflight checks.", c.args.NameOrUUID)
+		details := utils.EnvironmentPreflightTable(environment)
+		text += fmt.Sprintf("\n%s\n", details)
+		c.logger.Infof(ctx, text)
+
+	} else {
+		c.logger.Infof(ctx, "Environment %q has been updated. Run `meroxa env describe %s` for status", environment.Name, environment.Name)
+	}


Suggested change

text := fmt.Sprintf("Environment %q could not be updated.", c.args.NameOrUUID)

details := utils.EnvironmentPreflightTable(environment)

text += fmt.Sprintf("\n%s\n", details)

c.logger.Infof(ctx, text)

} else if state == meroxa.EnvironmentStatePreflightError {

text := fmt.Sprintf("Environment %q could not be updated because it failed the preflight checks.", c.args.NameOrUUID)

details := utils.EnvironmentPreflightTable(environment)

text += fmt.Sprintf("\n%s\n", details)

c.logger.Infof(ctx, text)

} else {

c.logger.Infof(ctx, "Environment %q has been updated. Run `meroxa env describe %s` for status", environment.Name, environment.Name)

}

details := utils.EnvironmentPreflightTable(rr)

c.logger.Errorf(ctx,

"Environment %q could not be updated because it failed the preflight checks\n%s\n",

environment.Name,

details)

} else {

c.logger.Infof(ctx,

"Preflight checks have passed. Environment %q is being updated. Run `meroxa env describe %s` for status",

environment.Name,

environment.Name)

}

janelletavares and others added 8 commits December 19, 2021 17:48

Show common environment when no environment provided

49c96c4

Show both name and UUID for connector environment

bb85d29

Unhide environment flags

c0183ca

docs updates

44d29a0

remove unnecessary attribute assignments

Merge branch 'env-table-display' of https://github.com/meroxa/cli

2c574d6

Merge branch 'hidden_env_flags' of https://github.com/meroxa/cli

3917e6b

Preflight cli changes

f899587

progress after testing

57d2358

anna-cross force-pushed the preflight_cli_changes branch from 57d2358 to f899587 Compare February 7, 2022 22:14

anna-cross and others added 8 commits February 7, 2022 17:16

Merge branch 'master' of https://github.com/meroxa/cli

8e5159f

Preflight cli changes

b5c7793

...

d73b871

Merge branch 'preflight_cli_changes' of https://github.com/meroxa/cli …

2e0f419

…into preflight_cli_changes

more testing

bc3a263

merge

cec8101

with unit tests

2e77dfb

tweak

c817d63

janelletavares marked this pull request as ready for review February 8, 2022 00:11

janelletavares requested review from a team, owenthereal and ericcheatham and removed request for a team February 8, 2022 00:12

lint

ef6a1fe

janelletavares reviewed Feb 8, 2022

View reviewed changes

janelletavares added 3 commits February 8, 2022 11:50

tweaks

b2d535f

update meroxa-go after merge

a65f34f

state update

0374f07

janelletavares approved these changes Feb 8, 2022

View reviewed changes

Updating some cli displayes

b2e3a55

dianadoherty reviewed Feb 9, 2022

View reviewed changes

dianadoherty approved these changes Feb 9, 2022

View reviewed changes

anna-cross added 2 commits February 9, 2022 17:27

Cli desc changes

f1d2f2f

...

b452a7d

janelletavares changed the title ~~Preflight cli changes~~ feat(environments): Preflight cli changes Feb 9, 2022

anna-cross merged commit 75e2e88 into master Feb 9, 2022

anna-cross deleted the preflight_cli_changes branch February 9, 2022 23:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(environments): Preflight cli changes #246

feat(environments): Preflight cli changes #246

anna-cross commented Feb 7, 2022 •

edited

Loading

dianadoherty commented Feb 8, 2022

janelletavares Feb 8, 2022

janelletavares left a comment

anna-cross commented Feb 8, 2022

dianadoherty commented Feb 9, 2022 •

edited

Loading

dianadoherty Feb 9, 2022

dianadoherty Feb 9, 2022

dianadoherty Feb 9, 2022

dianadoherty Feb 9, 2022

feat(environments): Preflight cli changes #246

feat(environments): Preflight cli changes #246

Conversation

anna-cross commented Feb 7, 2022 • edited Loading

Description of change

Type of change

How was this tested?

Demo

Additional references

Documentation updated

dianadoherty commented Feb 8, 2022

janelletavares Feb 8, 2022

Choose a reason for hiding this comment

janelletavares left a comment

Choose a reason for hiding this comment

anna-cross commented Feb 8, 2022

dianadoherty commented Feb 9, 2022 • edited Loading

dianadoherty Feb 9, 2022

Choose a reason for hiding this comment

dianadoherty Feb 9, 2022

Choose a reason for hiding this comment

dianadoherty Feb 9, 2022

Choose a reason for hiding this comment

dianadoherty Feb 9, 2022

Choose a reason for hiding this comment

anna-cross commented Feb 7, 2022 •

edited

Loading

dianadoherty commented Feb 9, 2022 •

edited

Loading