search

GoogleCloudPlatform / testgrid

Apache License 2.0
194 stars 68 forks source link

Add a `GetDashboardTab` rpc method #1209

Open ankur12-1610 opened 1 year ago

ankur12-1610 commented 1 year ago

This PR adds a new rpc method namely GetDashboardTab which will further be converted to a new endpoint

google-oss-prow[bot] commented 1 year ago

Hi @ankur12-1610. Thanks for your PR.

I'm waiting for a GoogleCloudPlatform member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
ankur12-1610 commented 1 year ago

@chase @michelle192837 kindly review.

google-oss-prow[bot] commented 1 year ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ankur12-1610 Once this PR has been reviewed and has the lgtm label, please ask for approval from chases2. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/GoogleCloudPlatform/testgrid/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
ankur12-1610 commented 1 year ago

Thanks for the PR, @ankur12-1610 !

Michelle's comments are on spot. Broadly looking at this PR, I should probably clarify a couple of things about protos and API... Protos are easily extensible so please feel free to add the most important metadata fields first, iterate, and add the others as the need arises. They are both forward and backward compatible and we can even deprecate some filed if required.

API is an abstraction layer on top of the actual data (including the config proto) so we don't have to provide all the config metadata via API. Again, I think it would be useful to identify the most important info that the users might be interested in (e.g. gcs bucket location) and improve on that later

Thanks! for the info @sultan-duisenbay, I'll keep that in mind. I have added a follow-up commit that implements all the nits!

ankur12-1610 commented 1 year ago

Although tests might fail because I'm still working on writing the functions so the endpoint is not implemented yet! Will add a follow-up once I finish working on it.