Get chaos environments command #170

shivam-Purohit · 2023-12-11T06:14:53Z

refers issue #164

Addition of new commands :

litmusctl get chaos-environment --project-id="" --environment-id=""
litmusctl list chaos-environment --project-id=""

shivam-Purohit · 2023-12-11T14:27:20Z

Suggest if further changes are required.

shivam-Purohit · 2023-12-20T03:46:59Z

@SarthakJain26 @Saranya-jena can I get a review?

SarthakJain26 · 2023-12-22T11:18:45Z

pkg/cmd/get/environment.go

+		environmentList, err := environment.GetEnvironmentList(projectID, credentials);
+		if err != nil {
+			if strings.Contains(err.Error(), "permission_denied") {
+				utils.Red.Println("❌ The specified Project ID doesn't exist.")


Please update the message to, "You don't have enough permissions to access this resource"

SarthakJain26 · 2023-12-22T11:20:00Z

pkg/cmd/get/environment.go

+func init() { 
+	GetCmd.AddCommand(ChaosEnvironmentCmd);
+	ChaosEnvironmentCmd.Flags().String("project-id", "", "Set the project-id to list Chaos Environment from a particular project.")
+	ChaosEnvironmentCmd.Flags().String("environment-id", "", "Set the environment-id to list Chaos Environment")


Suggested change

ChaosEnvironmentCmd.Flags().String("environment-id", "", "Set the environment-id to list Chaos Environment")

ChaosEnvironmentCmd.Flags().String("environment-id", "", "Set the environment-id to get Chaos Environment")

SarthakJain26 · 2023-12-22T11:20:20Z

pkg/cmd/get/environment.go

+}
+func init() { 
+	GetCmd.AddCommand(ChaosEnvironmentCmd);
+	ChaosEnvironmentCmd.Flags().String("project-id", "", "Set the project-id to list Chaos Environment from a particular project.")


Suggested change

ChaosEnvironmentCmd.Flags().String("project-id", "", "Set the project-id to list Chaos Environment from a particular project.")

ChaosEnvironmentCmd.Flags().String("project-id", "", "Set the project-id to get Chaos Environment from a particular project.")

SarthakJain26 · 2023-12-22T11:21:09Z

pkg/cmd/list/environment.go

+		environmentList, err := environment.GetEnvironmentList(projectID, credentials);
+		if err != nil {
+			if strings.Contains(err.Error(), "permission_denied") {
+				utils.Red.Println("❌ The specified Project ID doesn't exist.")


Please update the message to, "You don't have enough permissions to access this resource"

SarthakJain26 · 2023-12-22T11:21:48Z

pkg/cmd/list/list.go

+var ListCmd = &cobra.Command{
+	Use: "list",
+	Short: `Examples:
+		#get list of chaos Chaos Experiments


Suggested change

#get list of chaos Chaos Experiments

#get list of chaos Chaos Environments

SarthakJain26 · 2023-12-22T11:23:37Z

@shivam-Purohit can you please also share the screen shots of the command and the output

SarthakJain26 · 2023-12-22T11:26:15Z

@shivam-Purohit please add these commands in the readme and usage docs as well

shivam-Purohit · 2023-12-29T05:14:58Z

@shivam-Purohit can you please also share the screen shots of the command and the output

sure!

shivam-Purohit · 2023-12-29T05:18:26Z

@shivam-Purohit please add these commands in the readme and usage docs as well

I added these in 0.23.0. Do we need to add in any other versions? i think we do not need that.

@SarthakJain26 I fixed all the changes you requested. Let me know if further changes are required.

SarthakJain26 · 2023-12-29T05:57:51Z

@shivam-Purohit , you have displayed the complete response coming from the API which is not a very good user experience. You will have to extract required fields and you can display them in table format. I have attached the screenshot for reference. You can display environmentID, name, createdAt and createdBy fields. Please update the PR accordingly

shivam-Purohit · 2024-01-04T05:10:20Z

hey @SarthakJain26 I am currently getting the response as null in many fields which ig should not. I tried rechecking the API. Is there something I am doing here to retrieve the data?

SarthakJain26 · 2024-01-04T09:00:14Z

hey @SarthakJain26 I am currently getting the response as null in many fields which ig should not. I tried rechecking the API. Is there something I am doing here to retrieve the data?

@shivam-Purohit Possibly yes. You need to check on what is the graphql request you are sending and then what is the data you are expecting back. While decoding the response from the graphql server you need to check if the struct you are using to decode the response has the required fields or not.

shivam-Purohit · 2024-01-07T12:52:55Z

@SarthakJain26 the error was actually due to query itself which we were sending through api. I changed it a bit for the newcommand.
I also refactored the output and reused some other components for similarity and reusability.

SarthakJain26 · 2024-01-07T18:21:14Z

Thanks @shivam-Purohit, update output looks awesome

Nageshbansal · 2024-01-07T19:30:15Z

@SarthakJain26, do we need two separate commands for this, i.e., list and get? Maybe get should be enough only to show a list of all environments, as that is what we're doing for experiments and infra.

SarthakJain26 · 2024-01-09T05:15:05Z

@SarthakJain26, do we need two separate commands for this, i.e., list and get? Maybe get should be enough only to show a list of all environments, as that is what we're doing for experiments and infra.

@Nageshbansal , by convention, list command can be used to list the resources under the project with required info, and get command can be used to get additional info about a particular resource. We can follow the same in other places as well.
Adding to this @shivam-Purohit please let us know if you can add other details also in the get environment command in couple of days so that we can merge this PR and take it in this release.

shivam-Purohit · 2024-01-09T12:05:23Z

@SarthakJain26, do we need two separate commands for this, i.e., list and get? Maybe get should be enough only to show a list of all environments, as that is what we're doing for experiments and infra.

@Nageshbansal , by convention, list command can be used to list the resources under the project with required info, and get command can be used to get additional info about a particular resource. We can follow the same in other places as well. Adding to this @shivam-Purohit please let us know if you can add other details also in the get environment command in couple of days so that we can merge this PR and take it in this release.

type Environment struct {
	ProjectID     string          `json:"projectID"`
	EnvironmentID string          `json:"environmentID"`
	Name          string          `json:"name"`
	Description   *string         `json:"description"`
	Tags          []string        `json:"tags"`
	Type          EnvironmentType `json:"type"`
	CreatedAt     string          `json:"createdAt"`
	CreatedBy     *UserDetails    `json:"createdBy"`
	UpdatedBy     *UserDetails    `json:"updatedBy"`
	UpdatedAt     string          `json:"updatedAt"`
	IsRemoved     *bool           `json:"isRemoved"`
	InfraIDs      []string        `json:"infraIDs"`
}

these are the details we can get from the response. @SarthakJain26 let me know what details you want to add, imo we can add type, description or maybe updated at and updated by.

SarthakJain26 · 2024-01-09T12:09:30Z

@shivam-Purohit you can also add the InfraIDs along with what you mentioned so that it infras within the environment can be linked.

shivam-Purohit · 2024-01-09T12:43:07Z

@shivam-Purohit you can also add the InfraIDs along with what you mentioned so that it infras within the environment can be linked.

sure!

shivam-Purohit · 2024-01-09T14:17:17Z

i also added the environment type after the screenshot was taken. Switched the structure to row like as it was overflowing and difficult to manage in table format. Hope this helps @SarthakJain26

SarthakJain26 · 2024-01-11T13:10:18Z

@shivam-Purohit can you please fix the fmt issues due to which build pipeline is failing and also sign your commits so that DCO can pass. We can then merge the PR

shivam-Purohit · 2024-01-11T16:47:15Z

can you please fix the fmt issues due to which build pipeline is failing and also sign your commits so that DCO can pass. We can then merge the PR

should I squash the commits into one?

Nageshbansal · 2024-01-11T16:52:37Z

@shivam-Purohit Squashing the commit is not required ( it will be taken care of while merging) , for now please fix the fmt in go files ( you can use gofmt ./... command to format the go files)

Nageshbansal · 2024-01-11T16:58:42Z

For DCO, you can refer to : https://github.com/litmuschaos/litmus/blob/master/CONTRIBUTING.md#sign-your-work-with-developer-certificate-of-origin

Signed-off-by: Shivam Purohit <[email protected]>

shivam-Purohit · 2024-01-11T18:07:16Z

@Nageshbansal my first commit was unsigned i could not find any other way to sign it. So I merged it. If the workflows get approval i can test the changes i just made . I use gofmt -w . that should be able to get rid of the fmt error

shivam-Purohit · 2024-01-11T18:09:50Z

@shivam-Purohit can you please fix the fmt issues due to which build pipeline is failing and also sign your commits so that DCO can pass. We can then merge the PR

@SarthakJain26 can you run the workflow again ig this should be fine now at least the fmt error

Jonsy13 · 2024-01-12T06:45:45Z

pkg/cmd/get/environment.go

+			if environmentListData[i].EnvironmentID == environmentID {
+				intUpdateTime, err := strconv.ParseInt(environmentListData[i].UpdatedAt, 10, 64)
+				if err != nil {
+					fmt.Println("Error converting UpdatedAt to int64:", err)


utils.Red.Println

Jonsy13 · 2024-01-12T06:46:06Z

pkg/cmd/get/environment.go

+			utils.White_B.Print("\nEnter the Project ID: ")
+			fmt.Scanln(&projectID)
+
+			for projectID == "" {


Can we use if here?

I picked it from experiments get command so the same code is repeated there but doesn't make any sense to put for and then exit anyway. If would be a better choice

Jonsy13 · 2024-01-12T06:46:14Z

pkg/cmd/get/environment.go

+			utils.White_B.Print("\nEnter the Environment ID: ")
+			fmt.Scanln(&environmentID)
+
+			for environmentID == "" {


Can we use if here?

Jonsy13 · 2024-01-12T06:47:25Z

pkg/cmd/get/environment.go

+				updatedTime := time.Unix(intUpdateTime, 0).String()
+				intCreatedTime, err := strconv.ParseInt(environmentListData[i].CreatedAt, 10, 64)
+				if err != nil {
+					fmt.Println("Error converting CreatedAt to int64:", err)


utils.Red.Println

Saranya-jena · 2024-01-12T08:11:06Z

Please make the changes suggested by @Jonsy13 . Rest all looks good. 🚀

Signed-off-by: Shivam Purohit <[email protected]>

shivam-Purohit · 2024-01-12T09:05:31Z

@Jonsy13 @Saranya-jena I made the changes suggested you can take a look. Suggest if any further changes needs to be done.

SarthakJain26 · 2024-01-15T10:09:05Z

@shivam-Purohit please fix the build pipeline

Signed-off-by: Shivam Purohit <[email protected]>

shivam-Purohit · 2024-01-15T11:28:48Z

@SarthakJain26 fixed the fmt error. Hope it resolves the build workflow

shivam-Purohit · 2024-01-15T11:52:34Z

Looks like it worked!

shivam-Purohit marked this pull request as ready for review December 11, 2023 14:25

shivam-Purohit requested review from Jonsy13, Saranya-jena and SarthakJain26 as code owners December 11, 2023 14:25

shivam-Purohit mentioned this pull request Dec 19, 2023

Get chaos environments command #164

Closed

SarthakJain26 requested changes Dec 22, 2023

View reviewed changes

shivam-Purohit force-pushed the newcommand branch from 1be4745 to 2d8cd74 Compare January 7, 2024 12:47

SarthakJain26 approved these changes Jan 11, 2024

View reviewed changes

add get/list env commands to docs

5ceb2ac

Signed-off-by: Shivam Purohit <[email protected]>

shivam-Purohit force-pushed the newcommand branch from 99fb654 to 5ceb2ac Compare January 11, 2024 18:04

Jonsy13 reviewed Jan 12, 2024

View reviewed changes

Saranya-jena approved these changes Jan 12, 2024

View reviewed changes

modify print for error message

4e46383

Signed-off-by: Shivam Purohit <[email protected]>

fix:fmt error

ef5cc63

Signed-off-by: Shivam Purohit <[email protected]>

SarthakJain26 merged commit cf8c33a into litmuschaos:master Jan 15, 2024
4 checks passed

	ChaosEnvironmentCmd.Flags().String("environment-id", "", "Set the environment-id to list Chaos Environment")
	ChaosEnvironmentCmd.Flags().String("environment-id", "", "Set the environment-id to get Chaos Environment")

	ChaosEnvironmentCmd.Flags().String("project-id", "", "Set the project-id to list Chaos Environment from a particular project.")
	ChaosEnvironmentCmd.Flags().String("project-id", "", "Set the project-id to get Chaos Environment from a particular project.")

	#get list of chaos Chaos Experiments
	#get list of chaos Chaos Environments

Get chaos environments command #170

Get chaos environments command #170

Conversation

shivam-Purohit commented Dec 11, 2023

shivam-Purohit commented Dec 11, 2023

shivam-Purohit commented Dec 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SarthakJain26 commented Dec 22, 2023

SarthakJain26 commented Dec 22, 2023

shivam-Purohit commented Dec 29, 2023

shivam-Purohit commented Dec 29, 2023

SarthakJain26 commented Dec 29, 2023

shivam-Purohit commented Jan 4, 2024

SarthakJain26 commented Jan 4, 2024

shivam-Purohit commented Jan 7, 2024

SarthakJain26 commented Jan 7, 2024

Nageshbansal commented Jan 7, 2024 • edited Loading

SarthakJain26 commented Jan 9, 2024

shivam-Purohit commented Jan 9, 2024

SarthakJain26 commented Jan 9, 2024

shivam-Purohit commented Jan 9, 2024

shivam-Purohit commented Jan 9, 2024 • edited Loading

SarthakJain26 commented Jan 11, 2024

shivam-Purohit commented Jan 11, 2024

Nageshbansal commented Jan 11, 2024 • edited Loading

Nageshbansal commented Jan 11, 2024

shivam-Purohit commented Jan 11, 2024

shivam-Purohit commented Jan 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shivam-Purohit Jan 12, 2024 • edited Loading

Choose a reason for hiding this comment

Jonsy13 Jan 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Saranya-jena commented Jan 12, 2024

shivam-Purohit commented Jan 12, 2024 • edited Loading

SarthakJain26 commented Jan 15, 2024

shivam-Purohit commented Jan 15, 2024

shivam-Purohit commented Jan 15, 2024

Nageshbansal commented Jan 7, 2024 •

edited

Loading

shivam-Purohit commented Jan 9, 2024 •

edited

Loading

Nageshbansal commented Jan 11, 2024 •

edited

Loading

shivam-Purohit Jan 12, 2024 •

edited

Loading

Jonsy13 Jan 12, 2024 •

edited

Loading

shivam-Purohit commented Jan 12, 2024 •

edited

Loading