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.

Sign up for GitHub

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

Jump to bottom

Implementation of Summary/Aggregation API in Tekton results #642

Merged
merged 2 commits into from
Jan 29, 2024
Merged

Implementation of Summary/Aggregation API in Tekton results #642

merged 2 commits into from
Jan 29, 2024

Conversation

avinal
Copy link
Member

@avinal avinal commented Oct 18, 2023 •
edited
Loading

Changes

Added a new api endpoint for fetching summary and aggregation for list of records. Now you can directly get aggreagation data for a given list of records without fetching and processing on the client side.

@tekton-robot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@tekton-robot tekton-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 18, 2023
@tekton-robot tekton-robot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 18, 2023
@avinal avinal changed the title PoC for Summary/Aggregation API in tekton results DO NOT MERGE: PoC for Summary/Aggregation API in tekton results Oct 18, 2023
@tekton-robot tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 31, 2023
@avinal avinal force-pushed the avinal/summary-poc branch from ee41c8e to 353e533 Compare November 13, 2023 12:13
@tekton-robot tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 13, 2023
@avinal avinal marked this pull request as ready for review November 13, 2023 12:18
@tekton-robot tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 13, 2023
@tekton-robot tekton-robot requested a review from khrm November 13, 2023 12:18
@tekton-robot
Copy link

The following is the coverage report on the affected files.
Say /test pull-tekton-results-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/api/server/v1alpha2/lister/aggregator.go Do not exist 4.5%
pkg/api/server/v1alpha2/summary.go Do not exist 0.0%

@avinal avinal force-pushed the avinal/summary-poc branch from 353e533 to dd1fd69 Compare November 20, 2023 09:17
@tekton-robot
Copy link

The following is the coverage report on the affected files.
Say /test pull-tekton-results-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/api/server/v1alpha2/lister/aggregator.go Do not exist 4.4%
pkg/api/server/v1alpha2/summary.go Do not exist 0.0%

@avinal avinal force-pushed the avinal/summary-poc branch from dd1fd69 to 7b78f5c Compare November 20, 2023 09:51
@tekton-robot
Copy link

The following is the coverage report on the affected files.
Say /test pull-tekton-results-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/api/server/v1alpha2/lister/aggregator.go Do not exist 4.4%
pkg/api/server/v1alpha2/summary.go Do not exist 0.0%

@avinal
add Summary proto definition
0d441f4 
Signed-off-by: Avinal Kumar <[email protected]>
@avinal avinal force-pushed the avinal/summary-poc branch from 7b78f5c to 124cbcf Compare November 21, 2023 09:02
@tekton-robot
Copy link

The following is the coverage report on the affected files.
Say /test pull-tekton-results-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/api/server/v1alpha2/lister/aggregator.go Do not exist 4.4%
pkg/api/server/v1alpha2/summary.go Do not exist 0.0%

sayan-biswas
sayan-biswas requested changes Nov 28, 2023
View reviewed changes
Copy link
Contributor

@sayan-biswas sayan-biswas left a comment

Choose a reason for hiding this comment

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

Generally looks good, minor code changes required along with godoc annotations and unit tests.

proto/v1alpha2/resources.proto Outdated Show resolved Hide resolved
proto/v1alpha2/api.proto Outdated Show resolved Hide resolved
pkg/api/server/v1alpha2/summary.go Show resolved Hide resolved
pkg/api/server/v1alpha2/summary.go Show resolved Hide resolved
pkg/api/server/v1alpha2/lister/aggregator.go Outdated Show resolved Hide resolved
pkg/api/server/v1alpha2/lister/aggregator.go Outdated Show resolved Hide resolved
pkg/api/server/v1alpha2/lister/aggregator.go Outdated Show resolved Hide resolved
pkg/api/server/v1alpha2/lister/aggregator.go Outdated Show resolved Hide resolved
@avinal avinal force-pushed the avinal/summary-poc branch from 124cbcf to 50b33b7 Compare December 6, 2023 08:06
@tekton-robot
Copy link

The following is the coverage report on the affected files.
Say /test pull-tekton-results-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/api/server/v1alpha2/lister/aggregator.go Do not exist 17.3%
pkg/api/server/v1alpha2/summary.go Do not exist 0.0%

@avinal avinal force-pushed the avinal/summary-poc branch from 50b33b7 to 846f51a Compare December 6, 2023 12:02
@tekton-robot
Copy link

The following is the coverage report on the affected files.
Say /test pull-tekton-results-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/api/server/v1alpha2/lister/aggregator.go Do not exist 27.2%
pkg/api/server/v1alpha2/summary.go Do not exist 0.0%

@avinal avinal force-pushed the avinal/summary-poc branch from 846f51a to 6527a9e Compare December 7, 2023 09:39
@tekton-robot
Copy link

The following is the coverage report on the affected files.
Say /test pull-tekton-results-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/api/server/v1alpha2/lister/aggregator.go Do not exist 27.4%
pkg/api/server/v1alpha2/summary.go Do not exist 0.0%

@avinal avinal force-pushed the avinal/summary-poc branch from 6527a9e to cc12aa1 Compare December 7, 2023 13:50
@tekton-robot
Copy link

The following is the coverage report on the affected files.
Say /test pull-tekton-results-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/api/server/v1alpha2/lister/aggregator.go Do not exist 27.6%
pkg/api/server/v1alpha2/summary.go Do not exist 0.0%

@avinal avinal changed the title DO NOT MERGE: PoC for Summary/Aggregation API in tekton results Implementation for Summary/Aggregation API in tekton results Dec 18, 2023
@avinal avinal changed the title Implementation for Summary/Aggregation API in tekton results Implementation of Summary/Aggregation API in Tekton results Dec 18, 2023
@avinal avinal force-pushed the avinal/summary-poc branch from cc12aa1 to 5ba99cf Compare January 3, 2024 09:06
@tekton-robot
Copy link

The following is the coverage report on the affected files.
Say /test pull-tekton-results-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/api/server/v1alpha2/lister/aggregator.go Do not exist 28.0%
pkg/api/server/v1alpha2/summary.go Do not exist 0.0%

@vdemeester
Copy link
Member

@avinal needs a release-note entry I think 👼🏼
/approve

@tekton-robot tekton-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jan 23, 2024
@avinal @khrm
implementation for summary of list of records
5da12e9 
- returns aggregations: total, statuses, avg, max, min and total
  duration
- Grouped aggregations on namespace, pipeline and time duration
- ordered response based on fields
- added clusterrole permission entries
- added docs for summary and aggreagtion API

Signed-off-by: Avinal Kumar <[email protected]>
Co-authored-by: Khurram Baig <[email protected]>
@avinal avinal force-pushed the avinal/summary-poc branch from 5ba99cf to 5da12e9 Compare January 24, 2024 13:35
@tekton-robot
Copy link

The following is the coverage report on the affected files.
Say /test pull-tekton-results-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/api/server/v1alpha2/lister/aggregator.go Do not exist 28.0%
pkg/api/server/v1alpha2/summary.go Do not exist 0.0%

@sayan-biswas
Copy link
Contributor

/approve

@tekton-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sayan-biswas, vdemeester

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

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [sayan-biswas,vdemeester]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

1 similar comment
@tekton-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sayan-biswas, vdemeester

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

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [sayan-biswas,vdemeester]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

khrm
khrm reviewed Jan 29, 2024
View reviewed changes
Copy link
Contributor

@khrm khrm left a comment

Choose a reason for hiding this comment

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

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 29, 2024
@tekton-robot tekton-robot merged commit 456ba20 into tektoncd:main Jan 29, 2024
5 of 6 checks passed
@khrm
Copy link
Contributor

khrm commented Feb 1, 2024

/kind feature

@tekton-robot tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Feb 1, 2024
avinal added a commit to avinal/community that referenced this pull request Feb 28, 2024
@avinal
TEP-0148: mark as implmented
f2951f4 
- Implemented in tektoncd/results#642 and released in `v0.9.0`

Signed-off-by: Avinal Kumar <[email protected]>
@avinal avinal mentioned this pull request Feb 28, 2024
Merged
tekton-robot pushed a commit to tektoncd/community that referenced this pull request Mar 8, 2024
@avinal @tekton-robot
TEP-0148: mark as implmented
0cf2556 
- Implemented in tektoncd/results#642 and released in `v0.9.0`

Signed-off-by: Avinal Kumar <[email protected]>
@avinal avinal mentioned this pull request Jun 11, 2024
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@khrm khrm khrm left review comments

@sayan-biswas sayan-biswas sayan-biswas approved these changes

@alan-ghelardi alan-ghelardi Awaiting requested review from alan-ghelardi

@enarha enarha Awaiting requested review from enarha

Assignees

@khrm khrm

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@avinal @tekton-robot @vdemeester @sayan-biswas @khrm