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

[PF-691] Add Location column in Cloud Environments page #2442

Merged
merged 4 commits into from
Apr 20, 2021

Conversation

wnojopra
Copy link
Contributor

@wnojopra wnojopra commented Apr 9, 2021

In DataBiosphere/leonardo#1933, zone and region parameters were added to createRuntime so Leo can now create instances outside of us-central1. In #2441, the Terra UI will allow use those new parameters.

This PR will updates the cloud enviroments page to show you the region/zone your environments are in.

Tested with a GCE instance and Dataproc cluster on both the default location (us-central1) and non-default locations.
image
image
image

Discussion of UI changes related to locality have been happening in this doc, including comments from UI team.

@wnojopra wnojopra requested a review from a team April 9, 2021 19:06
@jdcanas jdcanas self-requested a review April 16, 2021 20:44
@jdcanas jdcanas self-assigned this Apr 16, 2021
Copy link
Contributor

@jdcanas jdcanas left a comment

Choose a reason for hiding this comment

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

Looks good. I ran the tests locally and everything passed. They will not pass here because a forked repo does not play well with how the CI expects to auth.

Copy link
Contributor

@kyuksel kyuksel left a comment

Choose a reason for hiding this comment

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

👍🏼 with a nit.

Tested against dev backend.

headerRenderer: () => 'Location',
cellRenderer: ({ rowIndex }) => {
const { runtimeConfig: { zone, region } } = filteredRuntimes[rowIndex]
return zone ? zone : region
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit:

Suggested change
return zone ? zone : region
return zone || region

looks a bit more readable to me.

@wnojopra
Copy link
Contributor Author

Thanks @kyuksel and @jdcanas . Would one of you mind merging this? I don't have write access to this repo.

@kyuksel kyuksel merged commit 806d6c2 into DataBiosphere:dev Apr 20, 2021
@yonghaoy yonghaoy changed the title Add Location column in Cloud Environments page [PF-691] Add Location column in Cloud Environments page Apr 28, 2021
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

Successfully merging this pull request may close these issues.

3 participants