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

Wrong region displayed in message when no resources found #436

Closed
fernando-villalba opened this issue Jul 8, 2024 · 4 comments
Closed

Comments

@fernando-villalba
Copy link

Issue

When you run the following command:

 civo instance --region nyc1 ls

And no resources are found, the wrong region will display in the message, here are examples when default region set to LON1

image

This is because when issue 404 was implemented the region output is set to show the default as shown here:

image

Acceptance criteria

  • Ensure that the correct region shows on the output message when listing resources in a region where there are no resources.
@Praveen005
Copy link
Contributor

Hi Team,

I just looked into this issue and to solve it,

we can write a check in FinishAndPrintOutput() like below, as using civo instance --region nyc1 ls command will set common.RegionSet to nyc1

       if len(ow.Values) == 0 {
                var region string
                if common.RegionSet != ""{
                    region = common.RegionSet
                }else{
		    region = config.Current.Meta.DefaultRegion
		}
		fmt.Fprintf(os.Stderr, "No resources found in region %s. For a list of regions use the command 'civo region ls'\n", region)
	}

I kindly request your review.

Regards,

Praveen

@haardikdharma10
Copy link
Contributor

That looks good to me @Praveen005. Do you want to open a PR with that change along with a screenshot that the command works and i'd be happy to approve and merge.

@Praveen005
Copy link
Contributor

Hey, @haardikdharma10 ,

when I run the command it doesn't go through, since I don't have an active civo account. Can you kindly test the changes on your end?

Screenshot 2024-07-11 125143

Thank you.

@haardikdharma10
Copy link
Contributor

@Praveen005 - I tested the change and it works as expected. I have merged the PR and closing this issue. The change should be visible when we release our next version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants